diff --git a/supacode/Clients/Git/GitClient.swift b/supacode/Clients/Git/GitClient.swift index 797c4869..7fc0374e 100644 --- a/supacode/Clients/Git/GitClient.swift +++ b/supacode/Clients/Git/GitClient.swift @@ -496,10 +496,10 @@ struct GitClient { if probedByteCount < binaryProbeByteCount { let remainingProbeCount = binaryProbeByteCount - probedByteCount let probe = chunk.prefix(remainingProbeCount) - if probe.contains(0x00) { return nil } + if probe.containsByte(0x00) { return nil } probedByteCount += probe.count } - lineCount += chunk.reduce(0) { $0 + ($1 == 0x0A ? 1 : 0) } + lineCount += chunk.countOccurrences(of: 0x0A) lastByte = chunk.last } @@ -1407,3 +1407,39 @@ struct GitClient { } } + +/// Byte scans over `Data`'s contiguous storage. +/// +/// `Data` conforms to `Sequence`, so `reduce` and `contains` walk it through +/// `Data.Iterator` with a value-witness call per byte. Sampling a running instance +/// for 300 s attributed roughly one whole core to exactly that path inside +/// `countLines` — 31% in `Sequence.reduce`, 30% in `Data.Iterator.next`, 22% in +/// value witnesses — about 72% of everything the process was burning, while +/// counting lines in untracked files. `memchr` covers the same bytes in one +/// vectorized pass over contiguous memory. +extension Data { + /// Occurrences of `byte` in the whole buffer. + fileprivate nonisolated func countOccurrences(of byte: UInt8) -> Int { + withUnsafeBytes { raw -> Int in + guard let base = raw.baseAddress, !raw.isEmpty else { return 0 } + var count = 0 + var scanned = 0 + while scanned < raw.count, + let hit = memchr(base + scanned, Int32(byte), raw.count - scanned) + { + // memchr returns the hit itself, so resume one byte past it. + scanned = base.distance(to: UnsafeRawPointer(hit)) + 1 + count += 1 + } + return count + } + } + + /// Whether `byte` occurs anywhere in the buffer. Stops at the first hit. + fileprivate nonisolated func containsByte(_ byte: UInt8) -> Bool { + withUnsafeBytes { raw -> Bool in + guard let base = raw.baseAddress, !raw.isEmpty else { return false } + return memchr(base, Int32(byte), raw.count) != nil + } + } +} diff --git a/supacodeTests/GitClientLineChangesTests.swift b/supacodeTests/GitClientLineChangesTests.swift index fa1a62e8..99a7fcb7 100644 --- a/supacodeTests/GitClientLineChangesTests.swift +++ b/supacodeTests/GitClientLineChangesTests.swift @@ -312,6 +312,62 @@ struct GitClientLineChangesTests { #expect(count == 2) } + /// The reader works in 64 KiB chunks, so a scan that resumed from the wrong offset + /// after a hit — or restarted per chunk — would miscount only once a file is larger + /// than one chunk. Every other case in this file fits in a single chunk. + @Test func countLinesInFilesCountsAcrossChunkBoundaries() throws { + let fileManager = FileManager.default + let tempRoot = fileManager.temporaryDirectory.appending(path: UUID().uuidString) + defer { try? fileManager.removeItem(at: tempRoot) } + try fileManager.createDirectory(at: tempRoot, withIntermediateDirectories: true) + // 40_000 lines of "line\n" is ~200 KB, spanning four chunks. + let content = String(repeating: "line\n", count: 40_000) + try content.write(to: tempRoot.appending(path: "big.txt"), atomically: true, encoding: .utf8) + + #expect(GitClient.countLinesInFiles(["big.txt"], relativeTo: tempRoot) == 40_000) + } + + /// A newline landing on the final byte of a chunk is the boundary case: the scan must + /// neither drop it nor double-count it against the next chunk's first byte. + @Test func countLinesInFilesCountsANewlineOnTheChunkBoundary() throws { + let fileManager = FileManager.default + let tempRoot = fileManager.temporaryDirectory.appending(path: UUID().uuidString) + defer { try? fileManager.removeItem(at: tempRoot) } + try fileManager.createDirectory(at: tempRoot, withIntermediateDirectories: true) + let chunkByteCount = 64 * 1_024 + var bytes = Data(repeating: UInt8(ascii: "a"), count: chunkByteCount - 1) + bytes.append(0x0A) // exactly the last byte of chunk one + bytes.append(contentsOf: Data("second\n".utf8)) + try bytes.write(to: tempRoot.appending(path: "boundary.txt")) + + #expect(GitClient.countLinesInFiles(["boundary.txt"], relativeTo: tempRoot) == 2) + } + + /// Binary detection probes only the first 8 KiB. A NUL past that window has always + /// been counted as text, and the faster scan must not widen the probe by accident. + @Test func countLinesInFilesTreatsALateNULAsText() throws { + let fileManager = FileManager.default + let tempRoot = fileManager.temporaryDirectory.appending(path: UUID().uuidString) + defer { try? fileManager.removeItem(at: tempRoot) } + try fileManager.createDirectory(at: tempRoot, withIntermediateDirectories: true) + var bytes = Data(repeating: UInt8(ascii: "a"), count: 10_000) + bytes.append(0x00) // beyond the 8 KiB probe + bytes.append(0x0A) + try bytes.write(to: tempRoot.appending(path: "late-nul.txt")) + + #expect(GitClient.countLinesInFiles(["late-nul.txt"], relativeTo: tempRoot) == 1) + } + + @Test func countLinesInFilesCountsAnEmptyFileAsZero() throws { + let fileManager = FileManager.default + let tempRoot = fileManager.temporaryDirectory.appending(path: UUID().uuidString) + defer { try? fileManager.removeItem(at: tempRoot) } + try fileManager.createDirectory(at: tempRoot, withIntermediateDirectories: true) + try Data().write(to: tempRoot.appending(path: "empty.txt")) + + #expect(GitClient.countLinesInFiles(["empty.txt"], relativeTo: tempRoot) == 0) + } + private func writeGitIndexHeader( version: UInt32 = 2, entryCount: UInt32, -- 2.51.2 From 978b7b59023835560982b8db5a072d2e0cc3bb72 Mon Sep 17 00:00:00 2001 From: Simon Heimlicher Date: Fri, 31 Jul 2026 17:16:21 +0200 Subject: [PATCH 02/31] Skip untracked files too large to be hand-written `lineChanges` counts untracked lines for the sidebar diff badge by reading every untracked file end to end. The only early exit is a binary probe, so a file with no NUL byte in its first 8 KiB is read in full on every refresh regardless of size. A 35 MB `sample(1)` capture sitting in a working tree therefore cost 35 MB of reads per refresh to produce a number nobody wants: the badge reports the lines you are adding, and no one typed a profiling capture. Skip files at or above 2 MiB, checked through `resourceValues` before the handle is opened, so an oversized file costs a stat rather than a full read. `nil` already means "not worth counting" on this path, which is how binary files reach the same result, and the call site already coalesces it to zero. Source files sit orders of magnitude below the threshold. A `.gitignore` pattern was the alternative and is worse: it has to name the file, so it fixes one repository for one spelling and misses the next capture that lands under a different name. Two tests, each mutation-checked to fail only for its own case. Disabling the guard fails the oversized case; relaxing the comparison to exclusive fails it too, since the boundary is inclusive; and a file one byte under the limit must still be counted in full. --- supacode/Clients/Git/GitClient.swift | 22 ++++++++++++++ supacodeTests/GitClientLineChangesTests.swift | 30 +++++++++++++++++++ 2 files changed, 52 insertions(+) diff --git a/supacode/Clients/Git/GitClient.swift b/supacode/Clients/Git/GitClient.swift index 7fc0374e..8af3b03c 100644 --- a/supacode/Clients/Git/GitClient.swift +++ b/supacode/Clients/Git/GitClient.swift @@ -471,7 +471,29 @@ struct GitClient { data[offset..<(offset + 4)].reduce(UInt32(0)) { ($0 << 8) | UInt32($1) } } + /// Untracked files at or above this size contribute nothing to the diff badge. + /// + /// The badge counts the lines you are adding, and a file this large is not + /// something anyone typed. Profiling captures are the case that prompted it: + /// a single `sample(1)` output runs to tens of megabytes, and every byte of it + /// was being read on each refresh to produce a number no one wants. Source + /// files sit orders of magnitude below the threshold, so nothing a reader would + /// call a change is affected. + /// + /// Chosen over a `.gitignore` pattern deliberately. A pattern has to name the + /// file, so it fixes one repository for one spelling and misses the next + /// capture that lands under a different name. + nonisolated static let untrackedLineCountByteLimit = 2 * 1_024 * 1_024 + nonisolated private static func countLines(in fileURL: URL) -> Int? { + // Checked before opening the file, so an oversized one costs a stat rather + // than a full read. `nil` already means "not worth counting" here; binary + // files reach the same answer through the NUL probe below. + if let fileSize = try? fileURL.resourceValues(forKeys: [.fileSizeKey]).fileSize, + fileSize >= untrackedLineCountByteLimit + { + return nil + } guard let handle = try? FileHandle(forReadingFrom: fileURL) else { return nil } defer { try? handle.close() } diff --git a/supacodeTests/GitClientLineChangesTests.swift b/supacodeTests/GitClientLineChangesTests.swift index 99a7fcb7..ec5dfaf0 100644 --- a/supacodeTests/GitClientLineChangesTests.swift +++ b/supacodeTests/GitClientLineChangesTests.swift @@ -301,6 +301,36 @@ struct GitClientLineChangesTests { #expect(count == 2) } + /// A capture-sized untracked file must not be read at all. The badge counts + /// lines you are adding, and nobody typed 2 MiB of `sample(1)` output. + @Test func countLinesInFilesSkipsFilesAtOrAboveTheByteLimit() throws { + let fileManager = FileManager.default + let tempRoot = fileManager.temporaryDirectory.appending(path: UUID().uuidString) + defer { try? fileManager.removeItem(at: tempRoot) } + try fileManager.createDirectory(at: tempRoot, withIntermediateDirectories: true) + try "a\nb\n".write(to: tempRoot.appending(path: "small.txt"), atomically: true, encoding: .utf8) + // Text, not binary: the NUL probe would pass it and count every newline. + let oversized = Data(repeating: 0x0A, count: GitClient.untrackedLineCountByteLimit) + try oversized.write(to: tempRoot.appending(path: "sample.txt")) + + let count = GitClient.countLinesInFiles(["small.txt", "sample.txt"], relativeTo: tempRoot) + #expect(count == 2, "Only the small file should contribute") + } + + /// The boundary is the whole point of the guard, so pin the side that must + /// still count. One byte under the limit is an ordinary file. + @Test func countLinesInFilesStillCountsJustUnderTheByteLimit() throws { + let fileManager = FileManager.default + let tempRoot = fileManager.temporaryDirectory.appending(path: UUID().uuidString) + defer { try? fileManager.removeItem(at: tempRoot) } + try fileManager.createDirectory(at: tempRoot, withIntermediateDirectories: true) + let justUnder = Data(repeating: 0x0A, count: GitClient.untrackedLineCountByteLimit - 1) + try justUnder.write(to: tempRoot.appending(path: "big.txt")) + + let count = GitClient.countLinesInFiles(["big.txt"], relativeTo: tempRoot) + #expect(count == GitClient.untrackedLineCountByteLimit - 1) + } + @Test func countLinesInFilesSkipsMissingFiles() throws { let fileManager = FileManager.default let tempRoot = fileManager.temporaryDirectory.appending(path: UUID().uuidString) -- 2.51.2 From 616bbf4bf81b1b0c6b587b3ecbf807da1e6d3e62 Mon Sep 17 00:00:00 2001 From: Simon Heimlicher Date: Thu, 23 Jul 2026 06:52:29 +0200 Subject: [PATCH 03/31] Gate per-action TCA logging behind a launch flag The DEBUG action-logging reducer ran debugCaseOutput (Mirror reflection plus type-name demangling), snapshotted the entire app state, and deep- compared it on every single action. Stack sampling of an idle build with 24 terminal surfaces attributed a steady ~5% of the main thread to this path, with 93% of all reducer time on the main thread spent in the logging wrapper rather than the reducers themselves. Gate the whole block behind PROWL_LOG_TCA_ACTIONS (default off) so a normal Debug run pays none of it. When enabled, route the action label through SupaLogger.notice -> the unified log instead of print, so the stream is visible via 'make log-stream'; the Debug SupaLogger.debug prints to a stdout that a Finder/launchd-launched app discards. Add SupaLogger.notice (unified log in all configs) and LogActionsReducer pass-through tests. Document the flag in AGENTS.md. The markdown hook added blank lines before two pre-existing code fences in AGENTS.md. --- AGENTS.md | 4 ++ supacode/Support/DebugCaseOutput.swift | 20 +++++++- supacode/Support/SupaLogger.swift | 18 ++++--- supacodeTests/LogActionsReducerTests.swift | 55 ++++++++++++++++++++++ 4 files changed, 90 insertions(+), 7 deletions(-) create mode 100644 supacodeTests/LogActionsReducerTests.swift diff --git a/AGENTS.md b/AGENTS.md index acdcb897..97249077 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -21,6 +21,7 @@ make bump-version # Bump version (date-based YYYY.M.DD) and creat ``` Run a single test class or method: + ```bash xcodebuild test -project supacode.xcodeproj -scheme supacode -destination "platform=macOS" \ -only-testing:supacodeTests/TerminalTabManagerTests \ @@ -28,6 +29,7 @@ xcodebuild test -project supacode.xcodeproj -scheme supacode -destination "platf ``` **Swift Testing vs XCTest `-only-testing` format**: Swift Testing (`@Test`) requires trailing `()` in the test identifier. Without it, `xcodebuild` silently matches nothing and reports `TEST SUCCEEDED` with zero tests run. + ```bash # XCTest (func testFoo) -only-testing:supacodeTests/FooTests/testBar @@ -37,6 +39,8 @@ xcodebuild test -project supacode.xcodeproj -scheme supacode -destination "platf Requires [mise](https://mise.jdx.dev/) for zig, swiftlint, and xcsift tooling. +`make log-stream` shows no `TCA` action lines by default: per-action logging — the action label plus a full app-state snapshot and diff — is gated off because it runs on every action and shows up as steady main-thread cost. Launch with `PROWL_LOG_TCA_ACTIONS=1` (scheme env var, or exported before `open`) to trace the action stream through the unified log. + ## Architecture Prowl is a macOS orchestrator for running multiple coding agents in parallel, using GhosttyKit as the underlying terminal. diff --git a/supacode/Support/DebugCaseOutput.swift b/supacode/Support/DebugCaseOutput.swift index e5044ddb..294616c0 100644 --- a/supacode/Support/DebugCaseOutput.swift +++ b/supacode/Support/DebugCaseOutput.swift @@ -10,6 +10,18 @@ extension Reducer where State: Equatable { } } +/// When set, `LogActionsReducer` labels every action and logs it to the unified +/// log, plus prints a `CustomDump` state diff. Off by default: the label +/// reflection (`debugCaseOutput`) together with a full app-state snapshot and a +/// deep `==` compare run on *every* action, which stack sampling measured as a +/// steady main-thread cost under heavy action throughput. Enable per launch with +/// `PROWL_LOG_TCA_ACTIONS=1` (Xcode scheme env var, or `open` with the variable +/// exported) to trace the stream via `make log-stream`. +#if DEBUG + private let tcaActionLoggingEnabled = + ProcessInfo.processInfo.environment["PROWL_LOG_TCA_ACTIONS"] == "1" +#endif + struct LogActionsReducer: Reducer where Base.State: Equatable { let base: Base @@ -17,8 +29,14 @@ struct LogActionsReducer: Reducer where Base.State: Equatable { func reduce(into state: inout Base.State, action: Base.Action) -> Effect { #if DEBUG + guard tcaActionLoggingEnabled else { + return base.reduce(into: &state, action: action) + } let actionLabel = debugCaseOutput(action) - logger.debug("Action: \(actionLabel)") + // `notice`, not `debug`: in DEBUG `SupaLogger.debug` prints to a stdout + // that a Finder/launchd-launched app discards, so `make log-stream` would + // never see it. `notice` routes to the unified log in all configs. + logger.notice("Action: \(actionLabel)") let previousState = state let effects = base.reduce(into: &state, action: action) if previousState != state, let diff = CustomDump.diff(previousState, state) { diff --git a/supacode/Support/SupaLogger.swift b/supacode/Support/SupaLogger.swift index 9bc007df..e3ae252b 100644 --- a/supacode/Support/SupaLogger.swift +++ b/supacode/Support/SupaLogger.swift @@ -2,9 +2,7 @@ import OSLog nonisolated struct SupaLogger: Sendable { private let category: String - #if !DEBUG - private let logger: Logger - #endif + private let logger: Logger /// Signposter for emitting `os_signpost` intervals/events visible in /// Instruments. Signposts are essentially zero-cost when no Instruments /// session is attached (a single TLS read), so they are always live — @@ -23,9 +21,7 @@ nonisolated struct SupaLogger: Sendable { init(_ category: String) { self.category = category let subsystem = Bundle.main.bundleIdentifier ?? "com.onevcat.prowl" - #if !DEBUG - self.logger = Logger(subsystem: subsystem, category: category) - #endif + self.logger = Logger(subsystem: subsystem, category: category) self.signposter = OSSignposter(subsystem: subsystem, category: "PointsOfInterest") } @@ -53,6 +49,16 @@ nonisolated struct SupaLogger: Sendable { #endif } + /// Emits to the unified log (`log stream`, Console, `make log-stream`) in + /// every build configuration. Unlike `debug`/`info`, which `print` in DEBUG + /// so they surface in the Xcode console, `notice` is for opt-in diagnostics + /// that must be greppable from the unified log even during local development + /// — e.g. the gated TCA action stream, which a `print` to a discarded stdout + /// would never reach. + func notice(_ message: String) { + logger.notice("\(message, privacy: .public)") + } + /// Wraps `body` in an `os_signpost` interval named `name`. The /// interval renders as a labeled bar on the Instruments timeline, /// making it trivial to correlate hotspots with hangs/hitches without diff --git a/supacodeTests/LogActionsReducerTests.swift b/supacodeTests/LogActionsReducerTests.swift new file mode 100644 index 00000000..4fa04e36 --- /dev/null +++ b/supacodeTests/LogActionsReducerTests.swift @@ -0,0 +1,55 @@ +import ComposableArchitecture +import Testing + +@testable import supacode + +private struct Counter: Reducer { + struct State: Equatable { + var count = 0 + var label = "" + } + + enum Action: Equatable { + case increment + case setLabel(String) + } + + func reduce(into state: inout State, action: Action) -> Effect { + switch action { + case .increment: + state.count += 1 + return .none + case .setLabel(let value): + state.label = value + return .none + } + } +} + +@MainActor +struct LogActionsReducerTests { + /// With action logging off (the default), the wrapper must reduce exactly like + /// its base — same state mutation, no diverging behavior from the gated path. + @Test func passesActionsThroughToBaseWhenLoggingDisabled() { + let reducer = LogActionsReducer(base: Counter()) + var state = Counter.State() + + _ = reducer.reduce(into: &state, action: .increment) + _ = reducer.reduce(into: &state, action: .setLabel("repo")) + + #expect(state.count == 1) + #expect(state.label == "repo") + } + + /// A no-op action leaves state untouched, so the diff branch has nothing to + /// print; the reducer must still return the base's effect and state. + @Test func leavesStateUnchangedForActionsThatDoNotMutate() { + let reducer = LogActionsReducer(base: Counter()) + var state = Counter.State(count: 5, label: "keep") + + _ = reducer.reduce(into: &state, action: .setLabel("keep")) + + #expect(state.count == 5) + #expect(state.label == "keep") + } +} -- 2.51.2 From b2ac29364e510eb34d87ff10e39bc2174d6d9175 Mon Sep 17 00:00:00 2001 From: Simon Heimlicher Date: Thu, 23 Jul 2026 07:06:09 +0200 Subject: [PATCH 04/31] Memoize agent screen scans to skip re-parsing unchanged terminals A surface with a live-but-idle agent is polled every 300 ms, and each poll re-ran DetectedAgent.detectState over the full screen: line splitting, lowercasing, and per-agent heuristic scans. Stack sampling of an idle build with 24 surfaces attributed ~19% of the main thread to this path (detectClaude alone ~10%), all of it re-deriving the same result from unchanged text. Cache the last (agent, text, raw) scan per surface and reuse it while both the detected agent and the screen text are unchanged. detectState is a pure function of the screen, so reuse is exactly equivalent to recomputation. Time-based stabilization still runs every tick, so working-to-idle decay is unaffected. The cache is @ObservationIgnored (a pure memo, never drives the UI) and is torn down at every per-surface and global detection-cleanup site. The scan logic is a pure static helper (resolveRawState) with unit tests covering cold cache, reuse on match, and rescans on text or agent change. --- ...WorktreeTerminalState+AgentDetection.swift | 47 +++++++++++++--- .../Models/WorktreeTerminalState.swift | 17 ++++++ supacodeTests/AgentScreenScanCacheTests.swift | 53 +++++++++++++++++++ 3 files changed, 110 insertions(+), 7 deletions(-) create mode 100644 supacodeTests/AgentScreenScanCacheTests.swift diff --git a/supacode/Features/Terminal/Models/WorktreeTerminalState+AgentDetection.swift b/supacode/Features/Terminal/Models/WorktreeTerminalState+AgentDetection.swift index 4006f6f0..54908aaa 100644 --- a/supacode/Features/Terminal/Models/WorktreeTerminalState+AgentDetection.swift +++ b/supacode/Features/Terminal/Models/WorktreeTerminalState+AgentDetection.swift @@ -45,6 +45,7 @@ extension WorktreeTerminalState { agentDetectionPresenceBySurface.removeValue(forKey: surfaceID) lastWorkingAtBySurface.removeValue(forKey: surfaceID) lastAgentDetectionDiagnosticsBySurface.removeValue(forKey: surfaceID) + lastAgentScreenScanBySurface.removeValue(forKey: surfaceID) if surfaceAgentStates[surfaceID]?.detectedAgent == nil { surfaceAgentStates.removeValue(forKey: surfaceID) } @@ -89,13 +90,13 @@ extension WorktreeTerminalState { let now = Date() let previous = surfaceAgentStates[surfaceID] ?? PaneAgentState(lastChangedAt: now) let activeText = view.bridge.readActiveText() ?? "" - // `detectState` is a `nonisolated` pure function that runs in well under a - // millisecond on a terminal-sized active screen, so the prior `Task.detached` hop - // bought nothing but allocator churn. In long sessions, each detection - // tick (300 ms or 2 s per surface) was leaving a task stack + closure - // capture behind that never reached ARC; over a 24 h session this added - // up to hundreds of MB of unreferenced allocations. - let raw = agent.detectState(in: activeText) + // Reuse the previous scan while the screen and detected agent are unchanged. + // A live-but-idle agent is polled every 300 ms and `detectState` re-splits, + // lowercases, and scans the whole screen each time; skipping that for + // identical text is the bulk of steady-state detection cost. Time-based + // stabilization below still runs every tick, so working→idle decay is + // unaffected. + let raw = cachedRawState(forSurfaceID: surfaceID, agent: agent, text: activeText) guard surfaces[surfaceID] != nil else { return false } var lastWorkingAt = lastWorkingAtBySurface[surfaceID] @@ -174,6 +175,36 @@ extension WorktreeTerminalState { return true } + /// Resolves the raw agent state for `text`, reusing `cache` when it already + /// holds a scan for the same `agent` and identical `text`. Returns the raw + /// state and the scan to store back for the next call. + /// + /// `detectState` is a `nonisolated` pure function of the screen, so reusing + /// its result for identical input is exactly equivalent to recomputing it. + /// It runs inline (no `Task.detached`): the detached hop bought only allocator + /// churn — over a long session each tick left a task stack + closure capture + /// that never reached ARC, adding up to hundreds of MB of unreferenced + /// allocations. + nonisolated static func resolveRawState( + agent: DetectedAgent, + text: String, + cache: AgentScreenScan? + ) -> (raw: AgentRawState, scan: AgentScreenScan) { + if let cache, cache.agent == agent, cache.text == text { + return (cache.raw, cache) + } + let raw = agent.detectState(in: text) + return (raw, AgentScreenScan(agent: agent, text: text, raw: raw)) + } + + /// Instance wrapper over `resolveRawState` that reads and writes the per-surface + /// memo, keeping `detectAgentState` to a single line at the call site. + private func cachedRawState(forSurfaceID surfaceID: UUID, agent: DetectedAgent, text: String) -> AgentRawState { + let (raw, scan) = Self.resolveRawState(agent: agent, text: text, cache: lastAgentScreenScanBySurface[surfaceID]) + lastAgentScreenScanBySurface[surfaceID] = scan + return raw + } + private func resolvedLaunchObservation( identified: IdentifiedAgentProcess?, previous: PaneAgentState @@ -341,6 +372,7 @@ extension WorktreeTerminalState { agentDetectionPresenceBySurface.removeValue(forKey: surfaceId) lastWorkingAtBySurface.removeValue(forKey: surfaceId) lastAgentDetectionDiagnosticsBySurface.removeValue(forKey: surfaceId) + lastAgentScreenScanBySurface.removeValue(forKey: surfaceId) lastEmittedAgentEntriesBySurface.removeValue(forKey: surfaceId) onAgentEntryRemoved?(surfaceId) } @@ -356,6 +388,7 @@ extension WorktreeTerminalState { agentDetectionPresenceBySurface.removeAll() lastWorkingAtBySurface.removeAll() lastAgentDetectionDiagnosticsBySurface.removeAll() + lastAgentScreenScanBySurface.removeAll() lastEmittedAgentEntriesBySurface.removeAll() for id in removedIDs { onAgentEntryRemoved?(id) diff --git a/supacode/Features/Terminal/Models/WorktreeTerminalState.swift b/supacode/Features/Terminal/Models/WorktreeTerminalState.swift index ea133b34..7a44ad2f 100644 --- a/supacode/Features/Terminal/Models/WorktreeTerminalState.swift +++ b/supacode/Features/Terminal/Models/WorktreeTerminalState.swift @@ -61,6 +61,15 @@ final class WorktreeTerminalState { let isFocused: Bool } + /// One memoized agent-screen scan: the `raw` state `detectState` produced for + /// `text` under `agent`. Cached per surface so an unchanged screen is not + /// re-parsed on the next poll. + struct AgentScreenScan: Equatable { + let agent: DetectedAgent + let text: String + let raw: AgentRawState + } + let tabManager: TerminalTabManager let runtime: GhosttyRuntime let worktree: Worktree @@ -102,6 +111,14 @@ final class WorktreeTerminalState { var agentDetectionPresenceBySurface: [UUID: AgentDetectionPresence] = [:] var lastWorkingAtBySurface: [UUID: Date] = [:] var lastAgentDetectionDiagnosticsBySurface: [UUID: String] = [:] + /// Memoizes the last agent-screen scan per surface so `detectAgentState` can + /// reuse it while the terminal text and detected agent are unchanged. A + /// live-but-idle agent is polled every 300 ms; without this each poll re-ran + /// `DetectedAgent.detectState` — line splitting, lowercasing, and heuristic + /// scans over identical text. Observation-ignored: a pure cache that never + /// drives the UI. + @ObservationIgnored + var lastAgentScreenScanBySurface: [UUID: AgentScreenScan] = [:] /// Last `ActiveAgentEntry` emitted per surface. `detectAgentState` re-emits /// whenever any `PaneAgentState` field changes, including internal /// bookkeeping (raw-state oscillation, session miss streaks, presence diff --git a/supacodeTests/AgentScreenScanCacheTests.swift b/supacodeTests/AgentScreenScanCacheTests.swift new file mode 100644 index 00000000..08587c05 --- /dev/null +++ b/supacodeTests/AgentScreenScanCacheTests.swift @@ -0,0 +1,53 @@ +import Testing + +@testable import supacode + +struct AgentScreenScanCacheTests { + /// With no cache, the helper scans from scratch and returns a scan that + /// round-trips the inputs and the freshly computed raw state. + @Test func scansFromScratchWithoutCache() { + let (raw, scan) = WorktreeTerminalState.resolveRawState( + agent: .claude, + text: "screen", + cache: nil + ) + + #expect(raw == DetectedAgent.claude.detectState(in: "screen")) + #expect(scan == WorktreeTerminalState.AgentScreenScan(agent: .claude, text: "screen", raw: raw)) + } + + /// When the cached agent and text both match, the helper returns the cached + /// raw state without recomputing. Proven by seeding a sentinel raw a fresh + /// scan would never produce and asserting it comes back unchanged. + @Test func reusesCachedRawWhenAgentAndTextMatch() { + let text = "" + let sentinel = WorktreeTerminalState.AgentScreenScan(agent: .claude, text: text, raw: .blocked) + #expect(DetectedAgent.claude.detectState(in: text) != .blocked) + + let (raw, scan) = WorktreeTerminalState.resolveRawState(agent: .claude, text: text, cache: sentinel) + + #expect(raw == .blocked) + #expect(scan == sentinel) + } + + /// A changed screen invalidates the cache and forces a rescan. + @Test func rescansWhenTextChanges() { + let cache = WorktreeTerminalState.AgentScreenScan(agent: .claude, text: "old", raw: .blocked) + + let (raw, scan) = WorktreeTerminalState.resolveRawState(agent: .claude, text: "new", cache: cache) + + #expect(raw == DetectedAgent.claude.detectState(in: "new")) + #expect(scan.text == "new") + } + + /// A different detected agent invalidates the cache even when the text is + /// identical, since raw state is agent-specific. + @Test func rescansWhenAgentChanges() { + let cache = WorktreeTerminalState.AgentScreenScan(agent: .codex, text: "screen", raw: .blocked) + + let (raw, scan) = WorktreeTerminalState.resolveRawState(agent: .claude, text: "screen", cache: cache) + + #expect(raw == DetectedAgent.claude.detectState(in: "screen")) + #expect(scan.agent == .claude) + } +} -- 2.51.2 From 246373b6b3dd82fb9c742cd869aee9dd5b5bb0c6 Mon Sep 17 00:00:00 2001 From: Simon Heimlicher Date: Wed, 22 Jul 2026 12:56:32 +0200 Subject: [PATCH 05/31] Resolve agent rows through a cached worktree directory index The sidebar spent roughly half the app's CPU in SidebarListView.resolveWorktreeID. A 10s sample of a running instance with 6 agents and 34 terminal surfaces put 2253 of 4711 samples there, with stat and getattrlist among the top leaf frames. The cost was structural: activeAgentRowDisplays called resolveWorktreeID per agent row, and each call scanned every repository worktree, invoking PathPolicy.normalizeURL three times per candidate pair. normalizeURL performs a fileExists check plus resolvingSymlinksInPath, so every (row, worktree) pair cost two blocking filesystem round-trips inside a SwiftUI body getter that re-runs on every agent output tick. Replace the scan with WorktreeDirectoryIndex: candidate directories are normalized once into a dictionary keyed by joined path components, and a lookup walks the queried path upward until it hits an entry, so the deepest match wins as before. Keys compare whole components rather than string prefixes, so /tmp/repo cannot match a sibling /tmp/repo2. WorktreeDirectoryIndexCache memoizes the index against the id/directory pairs it was built from, so render passes that change only branch names, colors, or icons reuse it and touch the filesystem not at all. This also removes the delay between creating a worktree and seeing its row: insertWorktree updates state synchronously, and the same reducer case fires reloadRepositories, so both invalidations previously re-ran the full scan before the sidebar could paint. --- supacode/Domain/WorktreeDirectoryIndex.swift | 111 ++++++++++++++ .../Repositories/Views/SidebarListView.swift | 34 ++--- .../WorktreeDirectoryIndexTests.swift | 144 ++++++++++++++++++ 3 files changed, 268 insertions(+), 21 deletions(-) create mode 100644 supacode/Domain/WorktreeDirectoryIndex.swift create mode 100644 supacodeTests/WorktreeDirectoryIndexTests.swift diff --git a/supacode/Domain/WorktreeDirectoryIndex.swift b/supacode/Domain/WorktreeDirectoryIndex.swift new file mode 100644 index 00000000..2732e2ff --- /dev/null +++ b/supacode/Domain/WorktreeDirectoryIndex.swift @@ -0,0 +1,111 @@ +import Foundation +import IdentifiedCollections + +/// Maps a directory an agent runs in to the deepest repository or worktree that contains it. +/// +/// Normalizing a path to its canonical form touches the filesystem (a `stat` plus a symlink walk +/// per path component), so every candidate directory is normalized once when the index is built and +/// never again per lookup. A lookup then costs one normalization of the queried directory plus a +/// handful of dictionary probes, instead of re-scanning and re-normalizing every worktree. +nonisolated struct WorktreeDirectoryIndex: Equatable { + /// Keyed by normalized path components joined back together, so a probe compares whole components + /// rather than raw string prefixes — `/tmp/repo` must not match a sibling `/tmp/repo2`. + private var idsByNormalizedPath: [String: Worktree.ID] = [:] + private var deepestComponentCount = 0 + + init() {} + + init(repositories: some Sequence) { + for repository in repositories { + if repository.capabilities.supportsRunnableFolderActions, + !repository.capabilities.supportsWorktrees + { + insert(id: repository.id, directory: repository.rootURL) + } + for worktree in repository.worktrees { + insert(id: worktree.id, directory: worktree.workingDirectory) + } + } + } + + private mutating func insert(id: Worktree.ID, directory: URL) { + let components = PathPolicy.normalizeURL(directory).pathComponents + let key = Self.key(for: components) + // The first entry registered for a directory wins. This preserves the previous behavior when a + // plain-folder repository root and its main worktree resolve to the same path. + guard idsByNormalizedPath[key] == nil else { return } + idsByNormalizedPath[key] = id + deepestComponentCount = max(deepestComponentCount, components.count) + } + + /// Finds the most specific indexed directory containing `workingDirectory`. The walk starts at the + /// full path and shortens one component at a time, so the deepest match is the first hit. + func worktreeID(forWorkingDirectory workingDirectory: URL) -> Worktree.ID? { + guard !idsByNormalizedPath.isEmpty else { return nil } + var components = PathPolicy.normalizeURL(workingDirectory).pathComponents + // No indexed directory is deeper than this, so longer prefixes cannot match. + if components.count > deepestComponentCount { + components.removeLast(components.count - deepestComponentCount) + } + while !components.isEmpty { + if let id = idsByNormalizedPath[Self.key(for: components)] { + return id + } + components.removeLast() + } + return nil + } + + private static func key(for components: [String]) -> String { + components.joined(separator: "/") + } +} + +/// Memoizes the index across SwiftUI render passes. +/// +/// `SidebarListView.body` re-runs on every agent output tick, and rebuilding the index each time +/// would put one filesystem round-trip per worktree back on the main thread. The index is a pure +/// function of the repository set, so a cached copy stays valid until that set changes. +@MainActor +enum WorktreeDirectoryIndexCache { + private static var cachedSignature: [Entry] = [] + private static var cachedIndex = WorktreeDirectoryIndex() + + private struct Entry: Equatable { + let id: Worktree.ID + let directory: URL + } + + static func index(for repositories: IdentifiedArrayOf) -> WorktreeDirectoryIndex { + let signature = signature(for: repositories) + guard signature == cachedSignature else { + cachedSignature = signature + cachedIndex = WorktreeDirectoryIndex(repositories: repositories) + return cachedIndex + } + return cachedIndex + } + + /// The id/directory pairs the index is built from, in build order. Comparing these avoids + /// rebuilding when an unrelated part of a repository changes (a branch rename, a color, an icon). + private static func signature(for repositories: IdentifiedArrayOf) -> [Entry] { + var entries: [Entry] = [] + for repository in repositories { + if repository.capabilities.supportsRunnableFolderActions, + !repository.capabilities.supportsWorktrees + { + entries.append(Entry(id: repository.id, directory: repository.rootURL)) + } + for worktree in repository.worktrees { + entries.append(Entry(id: worktree.id, directory: worktree.workingDirectory)) + } + } + return entries + } + + /// Drops the memo so a test starts from a known state. + static func reset() { + cachedSignature = [] + cachedIndex = WorktreeDirectoryIndex() + } +} diff --git a/supacode/Features/Repositories/Views/SidebarListView.swift b/supacode/Features/Repositories/Views/SidebarListView.swift index ae36e49d..bc70debb 100644 --- a/supacode/Features/Repositories/Views/SidebarListView.swift +++ b/supacode/Features/Repositories/Views/SidebarListView.swift @@ -609,12 +609,16 @@ struct SidebarListView: View { repositories: IdentifiedArrayOf, metadata: ActiveAgentWorktreeMetadata ) -> [ActiveAgentEntry.ID: ActiveAgentRowDisplay] { + // Built (or reused) once for the whole batch: resolving each row against every worktree + // individually put a filesystem round-trip per row/worktree pair on the main thread. + let directoryIndex = WorktreeDirectoryIndexCache.index(for: repositories) var displays: [ActiveAgentEntry.ID: ActiveAgentRowDisplay] = [:] for entry in entries { displays[entry.id] = activeAgentRowDisplay( for: entry, repositories: repositories, - metadata: metadata + metadata: metadata, + directoryIndex: directoryIndex ) } return displays @@ -626,13 +630,17 @@ struct SidebarListView: View { /// 2. `workingDirectory` is known but outside every repo → derive a name from its last path /// component (same logic as adding a repository). /// 3. `workingDirectory` is unknown → fall back to the surface's owning worktree (legacy behavior). + /// `directoryIndex` is supplied by `activeAgentRowDisplays` so a batch shares one index; passing + /// `nil` builds a throwaway one for a single lookup. static func activeAgentRowDisplay( for entry: ActiveAgentEntry, repositories: IdentifiedArrayOf, - metadata: ActiveAgentWorktreeMetadata + metadata: ActiveAgentWorktreeMetadata, + directoryIndex: WorktreeDirectoryIndex? = nil ) -> ActiveAgentRowDisplay { if let workingDirectory = entry.workingDirectory { - if let key = resolveWorktreeID(forWorkingDirectory: workingDirectory, in: repositories) { + let index = directoryIndex ?? WorktreeDirectoryIndex(repositories: repositories) + if let key = index.worktreeID(forWorkingDirectory: workingDirectory) { let fallbackName = workingDirectory.lastPathComponent return ActiveAgentRowDisplay( repositoryName: metadata.repositoryNamesByWorktreeID[key] ?? fallbackName, @@ -665,24 +673,8 @@ struct SidebarListView: View { forWorkingDirectory workingDirectory: URL, in repositories: IdentifiedArrayOf ) -> Worktree.ID? { - var best: (id: Worktree.ID, depth: Int)? - func consider(id: Worktree.ID, directory: URL) { - guard PathPolicy.contains(workingDirectory, in: directory) else { return } - let depth = PathPolicy.normalizeURL(directory).pathComponents.count - if let current = best, current.depth >= depth { return } - best = (id, depth) - } - for repository in repositories { - if repository.capabilities.supportsRunnableFolderActions, - !repository.capabilities.supportsWorktrees - { - consider(id: repository.id, directory: repository.rootURL) - } - for worktree in repository.worktrees { - consider(id: worktree.id, directory: worktree.workingDirectory) - } - } - return best?.id + WorktreeDirectoryIndexCache.index(for: repositories) + .worktreeID(forWorkingDirectory: workingDirectory) } /// Directory of the surface's owning worktree, used when the agent hasn't diff --git a/supacodeTests/WorktreeDirectoryIndexTests.swift b/supacodeTests/WorktreeDirectoryIndexTests.swift new file mode 100644 index 00000000..7abdef32 --- /dev/null +++ b/supacodeTests/WorktreeDirectoryIndexTests.swift @@ -0,0 +1,144 @@ +import Foundation +import IdentifiedCollections +import Testing + +@testable import supacode + +@MainActor +struct WorktreeDirectoryIndexTests { + @Test func resolvesNestedDirectoryToItsEnclosingWorktree() { + let worktree = makeWorktree(repoRoot: "/tmp/repo", path: "/tmp/repo", branch: "main") + let index = WorktreeDirectoryIndex(repositories: [makeRepository(id: "/tmp/repo", worktrees: [worktree])]) + + #expect( + index.worktreeID(forWorkingDirectory: URL(fileURLWithPath: "/tmp/repo/src/lib")) == worktree.id + ) + } + + @Test func prefersTheDeepestContainingWorktree() { + let mainWorktree = makeWorktree(repoRoot: "/tmp/repo", path: "/tmp/repo", branch: "main") + let nestedWorktree = makeWorktree( + repoRoot: "/tmp/repo", + path: "/tmp/repo/worktrees/feature", + branch: "feature" + ) + let index = WorktreeDirectoryIndex( + repositories: [makeRepository(id: "/tmp/repo", worktrees: [mainWorktree, nestedWorktree])] + ) + + #expect( + index.worktreeID(forWorkingDirectory: URL(fileURLWithPath: "/tmp/repo/worktrees/feature/lib")) + == nestedWorktree.id + ) + #expect( + index.worktreeID(forWorkingDirectory: URL(fileURLWithPath: "/tmp/repo/src")) == mainWorktree.id + ) + } + + /// A sibling whose name merely starts with an indexed directory's name must not match. This is why + /// the index compares whole path components rather than raw string prefixes. + @Test func doesNotMatchSiblingDirectoryWithSharedNamePrefix() { + let worktree = makeWorktree(repoRoot: "/tmp/repo", path: "/tmp/repo", branch: "main") + let index = WorktreeDirectoryIndex(repositories: [makeRepository(id: "/tmp/repo", worktrees: [worktree])]) + + #expect(index.worktreeID(forWorkingDirectory: URL(fileURLWithPath: "/tmp/repo2/src")) == nil) + #expect(index.worktreeID(forWorkingDirectory: URL(fileURLWithPath: "/tmp/repo-backup")) == nil) + } + + @Test func resolvesPlainFolderRepositoryByItsRootURL() { + let repository = makeRepository(id: "/tmp/notes", kind: .plain, worktrees: []) + let index = WorktreeDirectoryIndex(repositories: [repository]) + + #expect( + index.worktreeID(forWorkingDirectory: URL(fileURLWithPath: "/tmp/notes/inbox")) == repository.id + ) + } + + @Test func returnsNilForDirectoryOutsideEveryRepository() { + let worktree = makeWorktree(repoRoot: "/tmp/repo", path: "/tmp/repo", branch: "main") + let index = WorktreeDirectoryIndex(repositories: [makeRepository(id: "/tmp/repo", worktrees: [worktree])]) + + #expect(index.worktreeID(forWorkingDirectory: URL(fileURLWithPath: "/tmp/scratch")) == nil) + } + + @Test func emptyIndexResolvesNothing() { + let index = WorktreeDirectoryIndex() + + #expect(index.worktreeID(forWorkingDirectory: URL(fileURLWithPath: "/tmp/repo")) == nil) + } + + /// Trailing slashes and `.` / `..` segments must normalize to the same lookup. + @Test func normalizesRelativeSegmentsBeforeMatching() { + let worktree = makeWorktree(repoRoot: "/tmp/repo", path: "/tmp/repo", branch: "main") + let index = WorktreeDirectoryIndex(repositories: [makeRepository(id: "/tmp/repo", worktrees: [worktree])]) + + #expect( + index.worktreeID(forWorkingDirectory: URL(fileURLWithPath: "/tmp/repo/src/../lib")) == worktree.id + ) + } + + // MARK: - Cache + + @Test func cacheRebuildsWhenAWorktreeIsAdded() { + WorktreeDirectoryIndexCache.reset() + let mainWorktree = makeWorktree(repoRoot: "/tmp/repo", path: "/tmp/repo", branch: "main") + let before: IdentifiedArrayOf = [makeRepository(id: "/tmp/repo", worktrees: [mainWorktree])] + #expect( + WorktreeDirectoryIndexCache.index(for: before) + .worktreeID(forWorkingDirectory: URL(fileURLWithPath: "/tmp/repo/worktrees/feature/lib")) + == mainWorktree.id + ) + + let nestedWorktree = makeWorktree( + repoRoot: "/tmp/repo", + path: "/tmp/repo/worktrees/feature", + branch: "feature" + ) + let after: IdentifiedArrayOf = [ + makeRepository(id: "/tmp/repo", worktrees: [mainWorktree, nestedWorktree]) + ] + + #expect( + WorktreeDirectoryIndexCache.index(for: after) + .worktreeID(forWorkingDirectory: URL(fileURLWithPath: "/tmp/repo/worktrees/feature/lib")) + == nestedWorktree.id + ) + } + + /// A branch rename changes the worktree's name but not its directory, so the memo stays valid. + @Test func cacheReturnsAnEquivalentIndexWhenOnlyBranchNamesChange() { + WorktreeDirectoryIndexCache.reset() + let original = makeWorktree(repoRoot: "/tmp/repo", path: "/tmp/repo", branch: "main") + let renamed = makeWorktree(repoRoot: "/tmp/repo", path: "/tmp/repo", branch: "trunk") + let first = WorktreeDirectoryIndexCache.index(for: [makeRepository(id: "/tmp/repo", worktrees: [original])]) + let second = WorktreeDirectoryIndexCache.index(for: [makeRepository(id: "/tmp/repo", worktrees: [renamed])]) + + #expect(first == second) + } + + // MARK: - Helpers + + private func makeWorktree(repoRoot: String, path: String, branch: String) -> Worktree { + Worktree( + id: path, + name: branch, + detail: branch, + workingDirectory: URL(fileURLWithPath: path), + repositoryRootURL: URL(fileURLWithPath: repoRoot) + ) + } + + private func makeRepository( + id: String, + kind: Repository.Kind = .git, + worktrees: IdentifiedArrayOf + ) -> Repository { + Repository( + id: id, + rootURL: URL(fileURLWithPath: id), + name: URL(fileURLWithPath: id).lastPathComponent, + kind: kind, + worktrees: worktrees + ) + } +} -- 2.51.2 From 7903375cc00f8ded583755e5ac36a14cd89c887b Mon Sep 17 00:00:00 2001 From: Simon Heimlicher Date: Thu, 23 Jul 2026 08:07:47 +0200 Subject: [PATCH 06/31] Stop agent-state churn from re-rendering the whole sidebar MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Diagnosis (instrumented build driven via the CLI, working agent, 8s): SidebarListView.body re-ran 9x and recreated every RepositorySectionView (99 child re-evals), driven by ActiveAgentsFeature.State.entries changing ~once per second. The entries churn comes from detection re-emitting an ActiveAgentEntry on every rawState flicker as an agent animates its spinner — a field no SwiftUI view renders (they show displayState; rawState is CLI-only). Two fixes: 1. Emission rate: dedup agent entries ignoring rawState, so a raw-only flicker no longer pushes a new entry into state or re-renders anything. rawState still rides along on entries that emit for a visible reason, so prowl agents reports it as of the last visible change. 2. Re-render scope: move the Active Agents panel and its entries read into SidebarActiveAgentsOverlay, so an entries change re-evaluates only that overlay — not SidebarListView.body and the repository list. The parent body no longer reads state.activeAgents.entries. Updates AgentEntryEmissionDedupTests to assert raw-only changes no longer emit. make build-app 0/0, make check clean, 36 affected tests pass. --- .../Models/ActiveAgentEntry.swift | 17 ++++- .../Views/SidebarActiveAgentsOverlay.swift | 71 +++++++++++++++++++ .../Repositories/Views/SidebarListView.swift | 47 ++++-------- ...WorktreeTerminalState+AgentDetection.swift | 5 +- .../AgentEntryEmissionDedupTests.swift | 14 ++-- 5 files changed, 110 insertions(+), 44 deletions(-) create mode 100644 supacode/Features/Repositories/Views/SidebarActiveAgentsOverlay.swift diff --git a/supacode/Features/ActiveAgents/Models/ActiveAgentEntry.swift b/supacode/Features/ActiveAgents/Models/ActiveAgentEntry.swift index d3f78358..46d2efb7 100644 --- a/supacode/Features/ActiveAgents/Models/ActiveAgentEntry.swift +++ b/supacode/Features/ActiveAgents/Models/ActiveAgentEntry.swift @@ -24,7 +24,10 @@ struct ActiveAgentEntry: Identifiable, Equatable, Sendable { let iconLookupToken: String let agent: DetectedAgent var session: AgentSession? - let rawState: AgentRawState + /// Un-stabilized per-poll detection result. Surfaced only by `prowl agents` + /// (`raw_state`); no SwiftUI view renders it (they show `displayState`). A + /// `var` so `equalsIgnoringRawState` can normalize it for emission dedup. + var rawState: AgentRawState let displayState: AgentDisplayState let lastChangedAt: Date /// Profile display name recorded at launch for Prowl-launched surfaces @@ -35,6 +38,18 @@ struct ActiveAgentEntry: Identifiable, Equatable, Sendable { launchProfileName ?? Self.displayName(iconLookupToken: iconLookupToken, agent: agent) } + /// Equality for emission purposes, excluding `rawState`. `rawState` oscillates + /// on every 300 ms poll while an agent animates its spinner; if it gated + /// emission, each flicker would push a new entry into `ActiveAgentsFeature` + /// state and re-render the whole sidebar for a value no view displays. + /// `rawState` still rides along on entries that emit for a visible reason, so + /// `prowl agents` reports the raw state as of the last visible change. + func equalsIgnoringRawState(_ other: ActiveAgentEntry) -> Bool { + var normalized = self + normalized.rawState = other.rawState + return normalized == other + } + /// The user-facing agent name: the launch command token (e.g. `omp`) when it /// maps to a known icon, else the semantic agent name (e.g. `pi`). Shared by /// the panel rows and the toolbar Agents capsule so both always agree. diff --git a/supacode/Features/Repositories/Views/SidebarActiveAgentsOverlay.swift b/supacode/Features/Repositories/Views/SidebarActiveAgentsOverlay.swift new file mode 100644 index 00000000..d4afa82a --- /dev/null +++ b/supacode/Features/Repositories/Views/SidebarActiveAgentsOverlay.swift @@ -0,0 +1,71 @@ +import ComposableArchitecture +import Sharing +import SwiftUI + +/// The Active Agents panel plus the `entries → row-display` computation that +/// feeds it, split out of `SidebarListView`. +/// +/// `ActiveAgentsFeature.State.entries` is re-emitted whenever an agent's state +/// changes — frequently while agents work. When the read of `entries` lived in +/// `SidebarListView.body`, every such change re-ran the entire sidebar body and +/// recreated every `RepositorySectionView` (measured at ~11 child re-evals per +/// parent re-eval). Isolating the `entries` read here means an entries change +/// re-evaluates only this overlay; the repository list is untouched. See +/// `docs-ai/032-performance-hardening`. +struct SidebarActiveAgentsOverlay: View { + @Bindable var store: StoreOf + let terminalManager: WorktreeTerminalManager + let panelHeight: Double + let maximumPanelHeight: Double + let panelOffset: Double + let isPanelHidden: Bool + let sidebarFooterHeight: Double + let onHeightChanged: (Double) -> Void + let onHeightChangeEnded: (Double) -> Void + + @Environment(\.resolvedKeybindings) private var resolvedKeybindings + @Environment(CommandKeyObserver.self) private var commandKeyObserver + @Shared(.repositoryAppearances) private var repositoryAppearances + + var body: some View { + let state = store.state + let metadata = SidebarListView.activeAgentWorktreeMetadata( + repositories: state.repositories, + customTitles: state.repositoryCustomTitles, + repositoryAppearances: repositoryAppearances + ) + let rowDisplays = SidebarListView.activeAgentRowDisplays( + entries: state.activeAgents.entries, + repositories: state.repositories, + metadata: metadata + ) + let selectedSurfaceID = state.selectedWorktreeID.flatMap { worktreeID in + terminalManager.stateIfExists(for: worktreeID)?.activeSurfaceID + } + // Only surface the hint while Cmd is held and the bindings are still at their + // defaults; a customized binding makes the merged "⌥⌃↑↓" glyph inaccurate. + let shortcutHint = + commandKeyObserver.isPressed + ? AppShortcuts.activeAgentsNavigationDisplay(in: resolvedKeybindings) + : nil + + ActiveAgentsPanel( + store: store.scope(state: \.activeAgents, action: \.activeAgents), + rowDisplays: rowDisplays, + selectedSurfaceID: selectedSurfaceID, + navigationShortcutHint: shortcutHint, + showTabTitles: state.showActiveAgentTabTitles, + height: panelHeight, + maximumHeight: maximumPanelHeight, + onHeightChanged: onHeightChanged, + onHeightChangeEnded: onHeightChangeEnded + ) + .padding(6) + .frame(height: panelHeight) + .offset(y: panelOffset) + .clipped() + .padding(.bottom, sidebarFooterHeight) + .allowsHitTesting(!isPanelHidden) + .animation(.easeOut(duration: 0.18), value: isPanelHidden) + } +} diff --git a/supacode/Features/Repositories/Views/SidebarListView.swift b/supacode/Features/Repositories/Views/SidebarListView.swift index ae36e49d..c68d2bc7 100644 --- a/supacode/Features/Repositories/Views/SidebarListView.swift +++ b/supacode/Features/Repositories/Views/SidebarListView.swift @@ -62,7 +62,6 @@ struct SidebarListView: View { @State private var isAddChoicePresented = false @Namespace private var topSegmentNamespace @Environment(\.resolvedKeybindings) private var resolvedKeybindings - @Environment(CommandKeyObserver.self) private var commandKeyObserver @Shared(.repositoryAppearances) private var repositoryAppearances var body: some View { @@ -72,31 +71,16 @@ struct SidebarListView: View { let expandableRepositoryIDs = Self.expandableRepositoryIDs(in: state.repositories) let repositoryItems = presentation.items.filter(\.isRepositoryOrderItem) let selectedWorktreeIDs = Self.selectedWorktreeIDs(in: state) - let selectedSurfaceID = state.selectedWorktreeID.flatMap { worktreeID in - terminalManager.stateIfExists(for: worktreeID)?.activeSurfaceID - } - // Only surface the hint while Cmd is held and the bindings are still at their - // defaults; a customized binding makes the merged "⌥⌃↑↓" glyph inaccurate. - let activeAgentsShortcutHint = - commandKeyObserver.isPressed - ? AppShortcuts.activeAgentsNavigationDisplay(in: resolvedKeybindings) - : nil let pendingSidebarReveal = state.pendingSidebarReveal let maximumPanelHeight = sidebarHeight > 0 ? ActiveAgentsFeature.maximumPanelHeight(forContainerHeight: sidebarHeight) : ActiveAgentsFeature.maximumPanelHeight - let agentWorktreeMetadata = Self.activeAgentWorktreeMetadata( - repositories: state.repositories, - customTitles: state.repositoryCustomTitles, - repositoryAppearances: repositoryAppearances - ) - let agentRowDisplays = Self.activeAgentRowDisplays( - entries: state.activeAgents.entries, - repositories: state.repositories, - metadata: agentWorktreeMetadata - ) + // The Active Agents panel and its `entries` read live in + // `SidebarActiveAgentsOverlay` so that agent-state churn re-evaluates only + // that overlay, not this body and the repository list. This body must not + // read `state.activeAgents.entries`. let panelHeight = min(resizingPanelHeight ?? state.activeAgents.panelHeight, maximumPanelHeight) let panelOffset = state.activeAgents.isPanelHidden ? panelHeight : 0 let activeAgentsPanelTopGap = 4.0 @@ -171,14 +155,14 @@ struct SidebarListView: View { } } .overlay(alignment: .bottom) { - ActiveAgentsPanel( - store: store.scope(state: \.activeAgents, action: \.activeAgents), - rowDisplays: agentRowDisplays, - selectedSurfaceID: selectedSurfaceID, - navigationShortcutHint: activeAgentsShortcutHint, - showTabTitles: state.showActiveAgentTabTitles, - height: panelHeight, - maximumHeight: maximumPanelHeight, + SidebarActiveAgentsOverlay( + store: store, + terminalManager: terminalManager, + panelHeight: panelHeight, + maximumPanelHeight: maximumPanelHeight, + panelOffset: panelOffset, + isPanelHidden: state.activeAgents.isPanelHidden, + sidebarFooterHeight: sidebarFooterHeight, onHeightChanged: { height in resizingPanelHeight = height }, @@ -187,13 +171,6 @@ struct SidebarListView: View { store.send(.activeAgents(.panelHeightChanged(height))) } ) - .padding(6) - .frame(height: panelHeight) - .offset(y: panelOffset) - .clipped() - .padding(.bottom, sidebarFooterHeight) - .allowsHitTesting(!state.activeAgents.isPanelHidden) - .animation(.easeOut(duration: 0.18), value: state.activeAgents.isPanelHidden) } .dropDestination(for: URL.self) { urls, _ in let fileURLs = urls.filter(\.isFileURL) diff --git a/supacode/Features/Terminal/Models/WorktreeTerminalState+AgentDetection.swift b/supacode/Features/Terminal/Models/WorktreeTerminalState+AgentDetection.swift index 4006f6f0..f11b0bba 100644 --- a/supacode/Features/Terminal/Models/WorktreeTerminalState+AgentDetection.swift +++ b/supacode/Features/Terminal/Models/WorktreeTerminalState+AgentDetection.swift @@ -281,7 +281,10 @@ extension WorktreeTerminalState { onAgentEntryRemoved?(surfaceID) return } - guard entry != lastEmittedAgentEntriesBySurface[surfaceID] else { return } + // Dedup ignoring `rawState`: it flickers every poll while an agent animates + // and drives no UI, so emitting on it alone would re-render the sidebar + // continuously. Visible changes (displayState, title, session, …) still emit. + guard lastEmittedAgentEntriesBySurface[surfaceID]?.equalsIgnoringRawState(entry) != true else { return } lastEmittedAgentEntriesBySurface[surfaceID] = entry onAgentEntryChanged?(entry) } diff --git a/supacodeTests/AgentEntryEmissionDedupTests.swift b/supacodeTests/AgentEntryEmissionDedupTests.swift index d6c550eb..480b8d28 100644 --- a/supacodeTests/AgentEntryEmissionDedupTests.swift +++ b/supacodeTests/AgentEntryEmissionDedupTests.swift @@ -11,25 +11,25 @@ import Testing /// changes so that churn never floods the terminal event stream. @MainActor struct AgentEntryEmissionDedupTests { - @Test func identicalEntryIsEmittedOnce() { + @Test func rawStateAndBookkeepingChangesDoNotReEmit() { let fixture = makeFixture() var received: [ActiveAgentEntry] = [] fixture.state.onAgentEntryChanged = { received.append($0) } - // Same visible entry, differing only in internal bookkeeping. + // Same visible entry; only rawState (fallbackState) and internal bookkeeping + // differ across the three emits. var paneState = PaneAgentState(detectedAgent: .claude, fallbackState: .working, state: .working) fixture.state.emitAgentEntry(surfaceID: fixture.pane.id, tabId: fixture.tabId, state: paneState) paneState.sessionMissStreak = 1 paneState.fallbackState = .idle fixture.state.emitAgentEntry(surfaceID: fixture.pane.id, tabId: fixture.tabId, state: paneState) - - // fallbackState is part of the visible entry (CLI raw_state); the streak - // alone must not re-emit. paneState.sessionMissStreak = 2 fixture.state.emitAgentEntry(surfaceID: fixture.pane.id, tabId: fixture.tabId, state: paneState) - #expect(received.count == 2) - #expect(received.map(\.rawState) == [.working, .idle]) + // rawState never renders in the sidebar, so a raw-only flicker must not + // re-emit; only the first (visible) entry is forwarded. + #expect(received.count == 1) + #expect(received.map(\.rawState) == [.working]) } @Test func visibleChangeStillEmits() { -- 2.51.2 From 219cf0f92d7553367f45c2349dbd5a3c311e1c34 Mon Sep 17 00:00:00 2001 From: Simon Heimlicher Date: Wed, 22 Jul 2026 13:02:27 +0200 Subject: [PATCH 07/31] Record the sidebar resolution fix and the swift-format PATH quirk MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add docs-ai/032-performance-hardening/003-sidebar-agent-row-resolution.md as wave 3 of the performance-hardening entry. The Active Agents row resolution repeated wave 1's isMainWorktree failure mode — path normalization per item inside a sidebar render pass — and compounded it by calling resolvingSymlinksInPath, which costs a syscall per path component. The amendment records the sampling evidence, the root cause, the index design, and a recurrence note for future sidebar-render code. Also document in AGENTS.md that make check fails with "swift-format: command not found" when the Xcode toolchain is not on PATH, along with the xcrun --find one-liner that resolves it. The markdown hook re-padded the 032 plan header table; the content is unchanged. --- AGENTS.md | 10 +++ docs-ai/032-performance-hardening/000-plan.md | 20 +++-- .../003-sidebar-agent-row-resolution.md | 88 +++++++++++++++++++ 3 files changed, 110 insertions(+), 8 deletions(-) create mode 100644 docs-ai/032-performance-hardening/003-sidebar-agent-row-resolution.md diff --git a/AGENTS.md b/AGENTS.md index acdcb897..39f59e7c 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -21,6 +21,7 @@ make bump-version # Bump version (date-based YYYY.M.DD) and creat ``` Run a single test class or method: + ```bash xcodebuild test -project supacode.xcodeproj -scheme supacode -destination "platform=macOS" \ -only-testing:supacodeTests/TerminalTabManagerTests \ @@ -28,6 +29,7 @@ xcodebuild test -project supacode.xcodeproj -scheme supacode -destination "platf ``` **Swift Testing vs XCTest `-only-testing` format**: Swift Testing (`@Test`) requires trailing `()` in the test identifier. Without it, `xcodebuild` silently matches nothing and reports `TEST SUCCEEDED` with zero tests run. + ```bash # XCTest (func testFoo) -only-testing:supacodeTests/FooTests/testBar @@ -112,6 +114,14 @@ Reducer ← .terminalEvent(Event) ← AsyncStream - SwiftLint runs in strict mode; never disable lint rules without permission - Custom SwiftLint rule: `store_state_mutation_in_views` — do not mutate `store.*` directly in view files; send actions instead - Before creating a PR, run `make check`. Use `make format` only for intentional full-tree formatting cleanup. +- If `make check` fails with `swift-format: command not found`, the Xcode toolchain is not on `PATH`. The Makefile invokes `swift-format` unqualified, and the binary ships inside Xcode rather than in a standard bin directory. Prepend it for the invocation: + + ```bash + export PATH="$(dirname "$(xcrun --find swift-format)"):$PATH" + make check + ``` + + `make lint` is unaffected — it already runs SwiftLint through `mise exec`. ## UX Standards diff --git a/docs-ai/032-performance-hardening/000-plan.md b/docs-ai/032-performance-hardening/000-plan.md index 9a0efd65..26c3ed05 100644 --- a/docs-ai/032-performance-hardening/000-plan.md +++ b/docs-ai/032-performance-hardening/000-plan.md @@ -1,13 +1,13 @@ # 032 — Performance Hardening: Plan -| | | -| --- | --- | -| **Status** | Implemented (retrospective) | -| **Anchor date** | 2026-05-21 | -| **Documented** | 2026-07-12 (backfilled) | -| **Primary PRs** | #231, #367, #371, #398 (wave 1); #414, #415, #416, #417 (wave 2, see amendment) | -| **Sources** | PR descriptions, Sentry App Hang evidence quoted in #231, `docs-ai/017-upstream-sync-process/upstream-ledger.md` (2026-06-09 batch) | -| **Related** | [020-observability](../020-observability/000-plan.md), [017-upstream-sync-process](../017-upstream-sync-process/000-plan.md), [028-pr-status-tracking](../028-pr-status-tracking/000-plan.md), [030-agent-status-detection](../030-agent-status-detection/000-plan.md) | +| | | +| --------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| **Status** | Implemented (retrospective) | +| **Anchor date** | 2026-05-21 | +| **Documented** | 2026-07-12 (backfilled) | +| **Primary PRs** | #231, #367, #371, #398 (wave 1); #414, #415, #416, #417 (wave 2, see amendment) | +| **Sources** | PR descriptions, Sentry App Hang evidence quoted in #231, `docs-ai/017-upstream-sync-process/upstream-ledger.md` (2026-06-09 batch) | +| **Related** | [020-observability](../020-observability/000-plan.md), [017-upstream-sync-process](../017-upstream-sync-process/000-plan.md), [028-pr-status-tracking](../028-pr-status-tracking/000-plan.md), [030-agent-status-detection](../030-agent-status-detection/000-plan.md) | ## Background @@ -94,3 +94,7 @@ Wave-1 fixes, each independent: - Updated 2026-06-08: wave 2 — four upstream-ported performance fixes for agent-hot paths (#414–#417, from the 2026-06-09 upstream review batch) — see [002-june-upstream-ports.md](002-june-upstream-ports.md) +- Updated 2026-07-22: wave 3 — the Active Agents row resolution repeated wave 1's + `isMainWorktree` failure mode and added blocking filesystem calls to it; replaced with a + cached `WorktreeDirectoryIndex` — see + [003-sidebar-agent-row-resolution.md](003-sidebar-agent-row-resolution.md) diff --git a/docs-ai/032-performance-hardening/003-sidebar-agent-row-resolution.md b/docs-ai/032-performance-hardening/003-sidebar-agent-row-resolution.md new file mode 100644 index 00000000..b5fe98a9 --- /dev/null +++ b/docs-ai/032-performance-hardening/003-sidebar-agent-row-resolution.md @@ -0,0 +1,88 @@ +# 032.003 — Sidebar Agent Row Resolution + +## Context + +Wave 1 of this entry fixed `RepositoriesFeature.State.isMainWorktree(_:)`, where +`URL.standardizedFileURL` ran `O(repos × worktrees²)` times per sidebar render on the main +thread (#231). The Active Agents panel later grew its own resolution path with the same +shape, and it went further by touching the filesystem rather than only decoding strings. + +A 10-second `sample(1)` of a running Debug build with 6 Codex sessions and 34 terminal +surfaces attributed 2253 of 4711 samples — roughly half of all process CPU — to +`SidebarListView.resolveWorktreeID(forWorkingDirectory:in:)`. The main thread was on-CPU for +4203 of those samples. `stat` and `__getattrlist` ranked among the top non-blocking leaf +frames, alongside Foundation's URL path-normalization internals +(`String._removingDotSegments`, `_hasDotDotComponent`, `_compressingSlashes`). + +The reported symptom was twofold: sustained high CPU whenever agents streamed output, and a +roughly one-second delay between creating a worktree and its row appearing in the sidebar. + +## Root cause + +`SidebarListView.activeAgentRowDisplays(entries:repositories:metadata:)` called +`resolveWorktreeID` once per agent row, and each call scanned every worktree of every +repository. Per candidate pair it invoked `PathPolicy.normalizeURL` three times — twice +inside `PathPolicy.contains(_:in:)` and once for the depth comparison. + +`PathPolicy.normalizeURL` (`supacode/Support/PathPolicy.swift`) performs a +`FileManager.fileExists` check followed by `resolvingSymlinksInPath()`, which issues one +`getattrlist` per path component. So every `(row, worktree)` pair cost two blocking +filesystem round-trips, inside a SwiftUI `body` getter that re-runs on every agent output +tick. + +The worktree-creation lag had the same origin rather than a slow `wt` subprocess. +`insertWorktree` mutates `state.repositories` synchronously +(`supacode/Features/Repositories/Reducer/RepositoriesFeature+WorktreeState.swift`), and the +same reducer case also fires `.reloadRepositories(animated:)` +(`supacode/Features/Repositories/Reducer/RepositoriesFeature+WorktreeCreation.swift`). Both +invalidations forced a full rescan before the sidebar could paint, and the new worktree +permanently lengthened the inner loop. + +## Change + +`supacode/Domain/WorktreeDirectoryIndex.swift` introduces two types: + +- `WorktreeDirectoryIndex` normalizes each candidate directory once into a dictionary keyed + by joined path components. A lookup normalizes the queried directory once, then walks it + upward one component at a time and returns the first hit, so the deepest containing + directory still wins. Keys compare whole components rather than raw string prefixes, which + keeps `/tmp/repo` from matching a sibling `/tmp/repo2`. +- `WorktreeDirectoryIndexCache` memoizes the index against the `(id, directory)` pairs it was + built from. Render passes that change only branch names, colors, or icons reuse it. + +`SidebarListView.activeAgentRowDisplays` now resolves the index once for the whole batch. +`resolveWorktreeID` is kept as the public entry point and delegates to the cache. + +Per sidebar render the cost drops from `agents × worktrees × 3` calls to `normalizeURL` — two +filesystem round-trips each — to zero filesystem calls while the repository set is unchanged, +and `worktrees + 1` on the render after it changes. + +Resolution semantics are unchanged, including the pre-existing asymmetry where a +non-existent path skips symlink resolution while an existing one does not. The 13 sidebar +tests in `supacodeTests/RepositorySectionViewTests.swift` pass unmodified. + +## Refs + +Commit `ddb203e6` on branch `perf/sidebar-worktree-resolution`. PR pending — the branch was +held for local verification before opening one. + +## Current state + +`supacode/Domain/WorktreeDirectoryIndex.swift` owns the resolution. Eight tests in +`supacodeTests/WorktreeDirectoryIndexTests.swift` cover nested resolution, deepest-match +preference, the sibling-prefix trap, plain-folder repositories, unmatched paths, the empty +index, `..` normalization, and both cache paths. + +`make build-app` reported 0 errors and 0 warnings, `make test` reported 1943 passing tests +and 0 failures, and `make check` (swift-format strict lint plus SwiftLint) was clean. + +The measured CPU reduction on a live instance is unverified: confirming it requires restarting +Prowl, which was deferred to avoid interrupting running agent sessions. + +## Recurrence note + +This is the third instance in this entry of the same failure mode: path normalization +performed per-item inside a sidebar render pass. `standardizedFileURL` and +`resolvingSymlinksInPath()` are not cheap accessors — the latter is a syscall per path +component. New sidebar-render code should resolve paths through a precomputed index rather +than normalizing inside a loop. -- 2.51.2 From 14a6c1d83cb512bc1861fb79086a6a9c83441039 Mon Sep 17 00:00:00 2001 From: Simon Heimlicher Date: Mon, 27 Jul 2026 09:51:56 +0200 Subject: [PATCH 08/31] Stop animated tab titles from rebuilding the whole tab bar A 300-second sample of a window with 32 tabs caught the main thread at 82.5% of a core: 47.8% in GraphHost.flushTransactions, 32.3% in the CoreAnimation commit (18.9% of it AppKit view-tree layout), against 2.4% for agent detection. TerminalTabItem.title is a var inside TerminalTabManager.tabs, and Observation tracks that array as a whole. An agent TUI animates a spinner glyph into its terminal title at roughly 10 Hz, so one pane's animation invalidated every view reading `tabs` - TerminalTabsRowView then rebuilt all 32 tabs and AppKit re-laid the window's view tree, twenty to thirty times a second. This is the same defect as the Active Agents emission churn, in a place where a rebuild costs far more. Space live title writes to at most one per tab per second, holding the newest suppressed title so a burst cannot queue up. A spinner that stops leaves no further write to carry its last frame, so the agent-detection poll flushes what is pending - no timer, and the poll already runs for exactly the panes that animate. Locked and custom titles behave as before; the existing no-op guard still handles a TUI re-emitting an unchanged title. Also drop the quadratic lookup this exposed: the row scanned manager.tabs with first(where:) inside a ForEach over that same array, so every rebuild cost 32x32 comparisons. Index it once per body instead. --- .../Terminal/Models/TerminalTabManager.swift | 77 +++++++++- ...WorktreeTerminalState+AgentDetection.swift | 5 + .../TabBar/Views/TerminalTabsRowView.swift | 10 +- .../TerminalTabTitleCoalescingTests.swift | 139 ++++++++++++++++++ 4 files changed, 228 insertions(+), 3 deletions(-) create mode 100644 supacodeTests/TerminalTabTitleCoalescingTests.swift diff --git a/supacode/Features/Terminal/Models/TerminalTabManager.swift b/supacode/Features/Terminal/Models/TerminalTabManager.swift index 98072a19..f2d32010 100644 --- a/supacode/Features/Terminal/Models/TerminalTabManager.swift +++ b/supacode/Features/Terminal/Models/TerminalTabManager.swift @@ -6,6 +6,11 @@ import Observation final class TerminalTabManager { var tabs: [TerminalTabItem] = [] { didSet { + // Only when tabs actually went away: this fires on every title write too, + // and the prune is O(n) where the guard is O(1). + if tabs.count < oldValue.count { + pruneTitleCoalescingState() + } guard let editingTabID, !tabs.contains(where: { $0.id == editingTabID }) else { return } self.editingTabID = nil } @@ -13,6 +18,23 @@ final class TerminalTabManager { var selectedTabId: TerminalTabID? private(set) var editingTabID: TerminalTabID? + /// Shortest gap between two live title writes for one tab. + /// + /// An agent TUI animates a spinner glyph inside its terminal title at roughly + /// 10 Hz. `tabs` is observed as a whole, so one tab's frame invalidates every + /// view reading the array — the tab bar then rebuilds every tab and AppKit + /// re-lays the window's view tree. Sampling a spike measured that at 47% of a + /// core in `flushTransactions` plus 32% in the CoreAnimation commit, against + /// 2% for agent detection. + static let liveTitleCoalescingInterval: TimeInterval = 1 + + /// Bookkeeping only — a write here must never invalidate the tab bar, which is + /// the whole point of the coalescing. + @ObservationIgnored private var lastLiveTitleWriteAt: [TerminalTabID: Date] = [:] + /// The most recent title held back by coalescing. A spinner that stops leaves + /// no further change to carry it, so `flushPendingTitles` lands it instead. + @ObservationIgnored private var pendingLiveTitles: [TerminalTabID: String] = [:] + /// Creates a tab next to the current selection. With `select: false` the /// selection is left untouched (background creation, e.g. a headless handoff /// launch) unless nothing was selected yet. @@ -45,17 +67,70 @@ final class TerminalTabManager { /// `displayTitle` actually changed (a custom title masks live updates), /// so callers can refresh derived UI like the Active Agents subtitle. @discardableResult - func updateTitle(_ id: TerminalTabID, title: String) -> Bool { + func updateTitle(_ id: TerminalTabID, title: String, now: Date = Date()) -> Bool { guard let index = tabs.firstIndex(where: { $0.id == id }) else { return false } guard !tabs[index].isTitleLocked else { return false } // A TUI re-emits the same title constantly; skip the no-op write so it // doesn't invalidate the tab bar while an agent streams output. guard tabs[index].title != title else { return false } + // A changed title arriving inside the interval is almost always the next + // frame of an animation. Hold it rather than rebuilding the tab bar for it; + // the newest one wins, so nothing queues up. + if let lastWriteAt = lastLiveTitleWriteAt[id], + now.timeIntervalSince(lastWriteAt) < Self.liveTitleCoalescingInterval + { + pendingLiveTitles[id] = title + return false + } + return writeLiveTitle(title, toTabAt: index, id: id, now: now) + } + + /// Lands any held-back title whose interval has elapsed. Returns the tabs whose + /// visible `displayTitle` moved, so callers can refresh derived UI exactly as + /// they would after `updateTitle`. + /// + /// Driven by the agent-detection poll rather than a timer: the only way a + /// pending title is left stranded is that the writes stopped, and the poll is + /// already running for precisely the panes that animate. + @discardableResult + func flushPendingTitles(now: Date = Date()) -> [TerminalTabID] { + guard !pendingLiveTitles.isEmpty else { return [] } + var changed: [TerminalTabID] = [] + for (id, title) in pendingLiveTitles { + guard let lastWriteAt = lastLiveTitleWriteAt[id], + now.timeIntervalSince(lastWriteAt) >= Self.liveTitleCoalescingInterval + else { continue } + pendingLiveTitles.removeValue(forKey: id) + guard let index = tabs.firstIndex(where: { $0.id == id }), + !tabs[index].isTitleLocked, + tabs[index].title != title + else { continue } + if writeLiveTitle(title, toTabAt: index, id: id, now: now) { + changed.append(id) + } + } + return changed + } + + private func writeLiveTitle( + _ title: String, + toTabAt index: Int, + id: TerminalTabID, + now: Date + ) -> Bool { + pendingLiveTitles.removeValue(forKey: id) + lastLiveTitleWriteAt[id] = now let previousDisplayTitle = tabs[index].displayTitle tabs[index].title = title return tabs[index].displayTitle != previousDisplayTitle } + private func pruneTitleCoalescingState() { + let liveIDs = Set(tabs.map(\.id)) + lastLiveTitleWriteAt = lastLiveTitleWriteAt.filter { liveIDs.contains($0.key) } + pendingLiveTitles = pendingLiveTitles.filter { liveIDs.contains($0.key) } + } + /// Sets (or clears, when blank) the user-defined title. Returns `true` when /// the visible `displayTitle` actually changed. @discardableResult diff --git a/supacode/Features/Terminal/Models/WorktreeTerminalState+AgentDetection.swift b/supacode/Features/Terminal/Models/WorktreeTerminalState+AgentDetection.swift index 4006f6f0..e6a61585 100644 --- a/supacode/Features/Terminal/Models/WorktreeTerminalState+AgentDetection.swift +++ b/supacode/Features/Terminal/Models/WorktreeTerminalState+AgentDetection.swift @@ -26,6 +26,11 @@ extension WorktreeTerminalState { guard let self, let view, self.surfaces[view.id] != nil else { return } let hasAgent = await self.detectAgentState(for: view, tabId: tabId) let now = Date() + // Lands the last frame of a spinner that stopped animating. Cheap when + // nothing is pending, which is the common case. + for flushedTabID in self.tabManager.flushPendingTitles(now: now) { + self.refreshAgentEntriesForTitleChange(in: flushedTabID) + } let schedule = self.agentDetectionSchedules[view.id] ?? .cold self.agentDetectionSchedules[view.id] = hasAgent ? schedule.observedAgent(now: now) : schedule.observedNoAgent(now: now) diff --git a/supacode/Features/Terminal/TabBar/Views/TerminalTabsRowView.swift b/supacode/Features/Terminal/TabBar/Views/TerminalTabsRowView.swift index 949a273f..1682bebc 100644 --- a/supacode/Features/Terminal/TabBar/Views/TerminalTabsRowView.swift +++ b/supacode/Features/Terminal/TabBar/Views/TerminalTabsRowView.swift @@ -21,10 +21,16 @@ struct TerminalTabsRowView: View { @State private var rowFrame: CGRect = .zero var body: some View { + // Read `tabs` once and index it: the lookup below runs inside a `ForEach` + // over the same collection, so scanning it per row made each rebuild + // quadratic — and a rebuild happens whenever any one tab's title changes. + let tabs = manager.tabs + let tabsByID = Dictionary(tabs.map { ($0.id, $0) }, uniquingKeysWith: { first, _ in first }) + ZStack(alignment: .topLeading) { HStack(alignment: .center, spacing: TerminalTabBarMetrics.tabSpacing) { ForEach(Array(openedTabs.enumerated()), id: \.element) { index, id in - if let item = manager.tabs.first(where: { $0.id == id }) { + if let item = tabsByID[id] { TerminalTabView( tab: item, isActive: manager.selectedTabId == id, @@ -65,7 +71,7 @@ struct TerminalTabsRowView: View { .simultaneousGesture(makeTabDragGesture(id: id)) .terminalTabContextMenu( tabId: id, - tabs: manager.tabs, + tabs: tabs, actions: TerminalTabContextMenuActions( renameTab: renameTab, changeIcon: changeIcon, diff --git a/supacodeTests/TerminalTabTitleCoalescingTests.swift b/supacodeTests/TerminalTabTitleCoalescingTests.swift new file mode 100644 index 00000000..0d6a1c94 --- /dev/null +++ b/supacodeTests/TerminalTabTitleCoalescingTests.swift @@ -0,0 +1,139 @@ +import Foundation +import Testing + +@testable import supacode + +/// `TerminalTabManager.tabs` is observed as a whole, so one tab's title write +/// rebuilds the entire tab bar. Agent TUIs animate a spinner glyph into the +/// title at roughly 10 Hz, so live title writes are spaced out; these tests pin +/// both the spacing and the trailing flush that keeps a settled title from +/// being stranded. +@MainActor +struct TerminalTabTitleCoalescingTests { + private let start = Date(timeIntervalSince1970: 1_000) + + private func makeManager() -> (TerminalTabManager, TerminalTabID) { + let manager = TerminalTabManager() + let id = manager.createTab(title: "initial", icon: nil) + return (manager, id) + } + + private func title(of manager: TerminalTabManager, _ id: TerminalTabID) -> String? { + manager.tabs.first(where: { $0.id == id })?.title + } + + @Test func theFirstTitleAfterAQuietPeriodIsWrittenImmediately() { + let (manager, id) = makeManager() + + #expect(manager.updateTitle(id, title: "building", now: start)) + #expect(title(of: manager, id) == "building") + } + + @Test func framesArrivingInsideTheIntervalDoNotReachTheTabs() { + let (manager, id) = makeManager() + _ = manager.updateTitle(id, title: "⠋ working", now: start) + + #expect(manager.updateTitle(id, title: "⠙ working", now: start.addingTimeInterval(0.1)) == false) + #expect(manager.updateTitle(id, title: "⠹ working", now: start.addingTimeInterval(0.2)) == false) + + #expect( + title(of: manager, id) == "⠋ working", + "Animation frames must not mutate the observed array" + ) + } + + @Test func aTitleArrivingAfterTheIntervalIsWrittenAgain() { + let (manager, id) = makeManager() + _ = manager.updateTitle(id, title: "⠋ working", now: start) + _ = manager.updateTitle(id, title: "⠙ working", now: start.addingTimeInterval(0.5)) + + #expect(manager.updateTitle(id, title: "⠸ working", now: start.addingTimeInterval(1.1))) + #expect(title(of: manager, id) == "⠸ working") + } + + /// Only the newest held-back title survives, so a burst never queues up. + @Test func flushLandsTheMostRecentSuppressedTitle() { + let (manager, id) = makeManager() + _ = manager.updateTitle(id, title: "⠋ working", now: start) + _ = manager.updateTitle(id, title: "⠙ working", now: start.addingTimeInterval(0.2)) + _ = manager.updateTitle(id, title: "⠸ working", now: start.addingTimeInterval(0.4)) + + #expect(manager.flushPendingTitles(now: start.addingTimeInterval(1.5)) == [id]) + #expect(title(of: manager, id) == "⠸ working") + } + + @Test func flushDoesNothingBeforeTheIntervalElapses() { + let (manager, id) = makeManager() + _ = manager.updateTitle(id, title: "⠋ working", now: start) + _ = manager.updateTitle(id, title: "⠙ working", now: start.addingTimeInterval(0.2)) + + #expect(manager.flushPendingTitles(now: start.addingTimeInterval(0.5)).isEmpty) + #expect(title(of: manager, id) == "⠋ working") + } + + @Test func flushIsANoOpWhenNothingWasSuppressed() { + let (manager, id) = makeManager() + _ = manager.updateTitle(id, title: "done", now: start) + + #expect(manager.flushPendingTitles(now: start.addingTimeInterval(5)).isEmpty) + #expect(title(of: manager, id) == "done") + } + + /// A suppressed frame must not resurface after a later title has been written, + /// or the tab would flick back to a stale animation frame. + @Test func aSuppressedTitleIsDiscardedOnceANewerOneIsWritten() { + let (manager, id) = makeManager() + _ = manager.updateTitle(id, title: "⠋ working", now: start) + _ = manager.updateTitle(id, title: "⠙ working", now: start.addingTimeInterval(0.2)) + _ = manager.updateTitle(id, title: "finished", now: start.addingTimeInterval(1.1)) + + #expect(manager.flushPendingTitles(now: start.addingTimeInterval(9)).isEmpty) + #expect(title(of: manager, id) == "finished") + } + + @Test func coalescingIsPerTabNotGlobal() { + let manager = TerminalTabManager() + let first = manager.createTab(title: "first", icon: nil) + let second = manager.createTab(title: "second", icon: nil) + + #expect(manager.updateTitle(first, title: "⠋ one", now: start)) + // The second tab has its own history, so its first write is not held back. + #expect(manager.updateTitle(second, title: "⠋ two", now: start.addingTimeInterval(0.1))) + + #expect(title(of: manager, first) == "⠋ one") + #expect(title(of: manager, second) == "⠋ two") + } + + @Test func aLockedTitleIsNeverWrittenOrHeld() { + let manager = TerminalTabManager() + let id = manager.createTab(title: "pinned", icon: nil, isTitleLocked: true) + + #expect(manager.updateTitle(id, title: "⠋ working", now: start) == false) + #expect(manager.updateTitle(id, title: "⠙ working", now: start.addingTimeInterval(0.2)) == false) + #expect(manager.flushPendingTitles(now: start.addingTimeInterval(5)).isEmpty) + #expect(title(of: manager, id) == "pinned") + } + + /// A custom title masks the live one, so the visible title never moves — the + /// live value still updates underneath so clearing the custom title reveals it. + @Test func aCustomTitleMasksTheFlushedLiveTitle() { + let (manager, id) = makeManager() + _ = manager.setCustomTitle(id, title: "my tab") + _ = manager.updateTitle(id, title: "⠋ working", now: start) + _ = manager.updateTitle(id, title: "⠙ working", now: start.addingTimeInterval(0.2)) + + #expect(manager.flushPendingTitles(now: start.addingTimeInterval(1.5)).isEmpty) + #expect(title(of: manager, id) == "⠙ working") + #expect(manager.tabs.first(where: { $0.id == id })?.displayTitle == "my tab") + } + + @Test func closingATabDropsItsCoalescingState() { + let (manager, id) = makeManager() + _ = manager.updateTitle(id, title: "⠋ working", now: start) + _ = manager.updateTitle(id, title: "⠙ working", now: start.addingTimeInterval(0.2)) + manager.closeTab(id) + + #expect(manager.flushPendingTitles(now: start.addingTimeInterval(5)).isEmpty) + #expect(manager.tabs.isEmpty) + } +} -- 2.51.2 From e77ba660b4961e2f9eea19c8465fe2d7078aa27b Mon Sep 17 00:00:00 2001 From: Simon Heimlicher Date: Tue, 28 Jul 2026 15:46:13 +0200 Subject: [PATCH 09/31] Pin every field the emission dedup must not ignore MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `equalsIgnoringRawState` normalizes one field and compares the rest through the synthesized `==`. Nothing structural stops a later edit from normalizing a second field, and the damage is silent: the helper still compiles, the existing dedup tests still pass, and the only symptom is that a real change to that field never reaches the sidebar. Two fields make that concrete. `session` carries the resume target `prowl agents` reports, and `workingDirectory` resolves the repository and branch the sidebar row displays — suppressing either leaves the UI describing state the agent has already left. Walk all fourteen fields, asserting each one alone breaks equality, plus the positive case that `rawState` is ignored. Mutation-checked: normalizing `session`, and separately `workingDirectory`, each fails only this test while the three existing dedup tests pass either way. --- .../AgentEntryEmissionDedupTests.swift | 97 +++++++++++++++++++ 1 file changed, 97 insertions(+) diff --git a/supacodeTests/AgentEntryEmissionDedupTests.swift b/supacodeTests/AgentEntryEmissionDedupTests.swift index 480b8d28..86c951a8 100644 --- a/supacodeTests/AgentEntryEmissionDedupTests.swift +++ b/supacodeTests/AgentEntryEmissionDedupTests.swift @@ -67,6 +67,103 @@ struct AgentEntryEmissionDedupTests { #expect(removed == [fixture.pane.id]) } + /// `equalsIgnoringRawState` normalizes one field and compares the rest through + /// the synthesized `==`. Nothing structural stops a later edit from + /// normalizing a second field, and the damage would be silent: the helper + /// still compiles, every test above still passes, and the only symptom is a + /// real change to that field never reaching the sidebar. `emitAgentEntry` + /// promises "displayState, title, session, …" still emit, so pin the whole + /// promise — one differing field must always be enough to break equality. + @Test func everyFieldExceptRawStateBreaksEmissionEquality() { + let base = Self.entry() + let variants: [(field: String, entry: ActiveAgentEntry)] = [ + ("id", Self.entry(id: Self.otherID)), + ("worktreeID", Self.entry(worktreeID: "/tmp/repo/other")), + ("worktreeName", Self.entry(worktreeName: "other")), + // Resolves the repository and branch the row displays, so a suppressed + // change leaves the sidebar naming the directory the agent has left. + ("workingDirectory", Self.entry(workingDirectory: URL(fileURLWithPath: "/tmp/repo/other"))), + ("workingDirectory=nil", Self.entry(workingDirectory: nil)), + ("tabID", Self.entry(tabID: Self.otherTabID)), + ("paneTitle", Self.entry(paneTitle: "other title")), + ("surfaceID", Self.entry(surfaceID: Self.otherID)), + ("paneIndex", Self.entry(paneIndex: 2)), + ("iconLookupToken", Self.entry(iconLookupToken: "codex")), + ("agent", Self.entry(agent: .codex)), + // Carries the resume target `prowl agents` reports; a suppressed change + // would keep pointing at the previous session's transcript. + ("session", Self.entry(session: Self.otherSession)), + ("session=nil", Self.entry(session: nil)), + ("displayState", Self.entry(displayState: .blocked)), + ("lastChangedAt", Self.entry(lastChangedAt: Date(timeIntervalSince1970: 5_000))), + ] + + for variant in variants { + #expect( + !base.equalsIgnoringRawState(variant.entry), + "A change to \(variant.field) must still emit; normalizing it would hide the change from the sidebar" + ) + } + + // The single field the helper exists to ignore. + #expect(base.equalsIgnoringRawState(Self.entry(rawState: .idle))) + } + + private static let baseID = UUID(uuidString: "00000000-0000-0000-0000-0000000000A1")! + private static let otherID = UUID(uuidString: "00000000-0000-0000-0000-0000000000A2")! + /// Fixed rather than freshly generated, so two entries under comparison differ + /// only in the field the case is about. + private static let baseTabID = TerminalTabID(rawValue: UUID(uuidString: "00000000-0000-0000-0000-0000000000B1")!) + private static let otherTabID = TerminalTabID(rawValue: UUID(uuidString: "00000000-0000-0000-0000-0000000000B2")!) + private static let baseSession = AgentSession( + id: "session-1", + transcriptPath: URL(fileURLWithPath: "/tmp/transcripts/session-1.jsonl"), + source: .openFile, + confidence: .exact + ) + private static let otherSession = AgentSession( + id: "session-2", + transcriptPath: URL(fileURLWithPath: "/tmp/transcripts/session-2.jsonl"), + source: .openFile, + confidence: .exact + ) + + /// Defaults spell out every field so a case can override exactly one and the + /// assertion stays about that field alone. + private static func entry( + id: UUID = baseID, + worktreeID: Worktree.ID = "/tmp/repo/worktree", + worktreeName: String = "worktree", + workingDirectory: URL? = URL(fileURLWithPath: "/tmp/repo/worktree"), + tabID: TerminalTabID = baseTabID, + paneTitle: String = "spx-h", + surfaceID: UUID = baseID, + paneIndex: Int = 1, + iconLookupToken: String = "claude", + agent: DetectedAgent = .claude, + session: AgentSession? = baseSession, + rawState: AgentRawState = .working, + displayState: AgentDisplayState = .working, + lastChangedAt: Date = Date(timeIntervalSince1970: 1_000) + ) -> ActiveAgentEntry { + ActiveAgentEntry( + id: id, + worktreeID: worktreeID, + worktreeName: worktreeName, + workingDirectory: workingDirectory, + tabID: tabID, + paneTitle: paneTitle, + surfaceID: surfaceID, + paneIndex: paneIndex, + iconLookupToken: iconLookupToken, + agent: agent, + session: session, + rawState: rawState, + displayState: displayState, + lastChangedAt: lastChangedAt + ) + } + private struct Fixture { let state: WorktreeTerminalState let tabId: TerminalTabID -- 2.51.2 From 08773383843c5f62cdb4cf592f5d4853508b9a16 Mon Sep 17 00:00:00 2001 From: Simon Heimlicher Date: Wed, 22 Jul 2026 22:44:27 +0200 Subject: [PATCH 10/31] Record the measured before/after profile for the sidebar resolution fix The entry claimed the CPU reduction was unverified. It has since been measured on a live instance: resolveWorktreeID is absent from the profile and SidebarListView.body fell from 51% to 2.4% of main-thread samples. --- .../003-sidebar-agent-row-resolution.md | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/docs-ai/032-performance-hardening/003-sidebar-agent-row-resolution.md b/docs-ai/032-performance-hardening/003-sidebar-agent-row-resolution.md index b5fe98a9..bc9cad5b 100644 --- a/docs-ai/032-performance-hardening/003-sidebar-agent-row-resolution.md +++ b/docs-ai/032-performance-hardening/003-sidebar-agent-row-resolution.md @@ -76,8 +76,21 @@ index, `..` normalization, and both cache paths. `make build-app` reported 0 errors and 0 warnings, `make test` reported 1943 passing tests and 0 failures, and `make check` (swift-format strict lint plus SwiftLint) was clean. -The measured CPU reduction on a live instance is unverified: confirming it requires restarting -Prowl, which was deferred to avoid interrupting running agent sessions. +The reduction was measured on a live instance after swapping in the fixed build. Two 10-second +`sample(1)` runs, before and after, on comparable workloads: + +| Main-thread measure | Before | After | +| ----------------------------- | ---------: | ---------: | +| `resolveWorktreeID` samples | 2253 (48%) | 0 (absent) | +| `SidebarListView.body` | 2382 (51%) | 183 (2.4%) | +| `GraphHost.flushTransactions` | 3774 (80%) | 1530 (20%) | +| Idle in `mach_msg2_trap` | ~11% | 53% | + +Process CPU fell from 94–134% to 29–70%. The workloads were not identical — 34 surfaces and 6 +sessions before versus 23 surfaces and 8 sessions after — so the percentages are indicative +rather than a controlled comparison; the disappearance of `resolveWorktreeID` from the profile +is not. The only residual frames are 24 samples (0.3%) in `WorktreeDirectoryIndex.worktreeID`, +which is the by-design single normalization of the queried directory per lookup. ## Recurrence note -- 2.51.2 From b77888f3e353213e43c0b6cfa13c32f3ae491d3a Mon Sep 17 00:00:00 2001 From: Simon Heimlicher Date: Mon, 27 Jul 2026 20:35:22 +0200 Subject: [PATCH 11/31] Make the tab-title coalescing tests able to fail MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A test-evidence audit found closingATabDropsItsCoalescingState proved nothing: it asserted flushPendingTitles returns empty after a close, but flushPendingTitles already skips any id missing from tabs on its own guard. Deleting pruneTitleCoalescingState outright — leaving bookkeeping that grows with every tab ever closed — left the test green. The dictionaries are private precisely so a title write cannot invalidate the tab bar, which also leaves the prune unobservable. Add a narrow read-only seam over the retained ids and assert those instead, plus a sibling case proving a surviving tab keeps its state. Two further gaps the same audit named: - The one-second interval was only ever exercised at convenient distances, so changing its `<` to `<=` passed all eleven tests. Pin both sides of the boundary. - The flush call site refreshed Active Agents entries inline in the detection Task, which no test could reach. Extract it as flushCoalescedTabTitles and cover it: a withheld title refreshes the entry when it lands, an empty flush emits nothing, and a flush inside the interval keeps holding. --- .../Terminal/Models/TerminalTabManager.swift | 12 +++ ...WorktreeTerminalState+AgentDetection.swift | 23 ++++- supacodeTests/TabTitleFlushRefreshTests.swift | 99 +++++++++++++++++++ .../TerminalTabTitleCoalescingTests.swift | 43 ++++++++ 4 files changed, 172 insertions(+), 5 deletions(-) create mode 100644 supacodeTests/TabTitleFlushRefreshTests.swift diff --git a/supacode/Features/Terminal/Models/TerminalTabManager.swift b/supacode/Features/Terminal/Models/TerminalTabManager.swift index f2d32010..4a1757ae 100644 --- a/supacode/Features/Terminal/Models/TerminalTabManager.swift +++ b/supacode/Features/Terminal/Models/TerminalTabManager.swift @@ -131,6 +131,18 @@ final class TerminalTabManager { pendingLiveTitles = pendingLiveTitles.filter { liveIDs.contains($0.key) } } + /// Every tab the coalescing bookkeeping still holds an entry for. + /// + /// The dictionaries are private so a title write is only ever observable through + /// `tabs`, which is what keeps them from invalidating the tab bar. That also hides + /// whether closing a tab pruned them: `flushPendingTitles` skips an absent tab on + /// its own existence guard, so it returns the same empty result either way. Without + /// this seam a test cannot tell a working prune from a leak that grows with every + /// tab ever closed. + var coalescedTabIDsForTesting: Set { + Set(lastLiveTitleWriteAt.keys).union(pendingLiveTitles.keys) + } + /// Sets (or clears, when blank) the user-defined title. Returns `true` when /// the visible `displayTitle` actually changed. @discardableResult diff --git a/supacode/Features/Terminal/Models/WorktreeTerminalState+AgentDetection.swift b/supacode/Features/Terminal/Models/WorktreeTerminalState+AgentDetection.swift index e6a61585..2ae73c40 100644 --- a/supacode/Features/Terminal/Models/WorktreeTerminalState+AgentDetection.swift +++ b/supacode/Features/Terminal/Models/WorktreeTerminalState+AgentDetection.swift @@ -26,11 +26,7 @@ extension WorktreeTerminalState { guard let self, let view, self.surfaces[view.id] != nil else { return } let hasAgent = await self.detectAgentState(for: view, tabId: tabId) let now = Date() - // Lands the last frame of a spinner that stopped animating. Cheap when - // nothing is pending, which is the common case. - for flushedTabID in self.tabManager.flushPendingTitles(now: now) { - self.refreshAgentEntriesForTitleChange(in: flushedTabID) - } + self.flushCoalescedTabTitles(now: now) let schedule = self.agentDetectionSchedules[view.id] ?? .cold self.agentDetectionSchedules[view.id] = hasAgent ? schedule.observedAgent(now: now) : schedule.observedNoAgent(now: now) @@ -270,6 +266,23 @@ extension WorktreeTerminalState { } } + /// Lands tab titles held back by coalescing and refreshes the Active Agents + /// entries that follow them — the same refresh a title written directly through + /// `updateTitle` triggers, so the two paths cannot drift. + /// + /// Driven by the detection poll: a spinner that stops animating leaves no further + /// title change to carry its last frame, and the poll is already running for + /// exactly the panes that animate. Split out of that `Task` loop because the loop + /// offers no synchronous seam a test can drive. + @discardableResult + func flushCoalescedTabTitles(now: Date = Date()) -> [TerminalTabID] { + let flushed = tabManager.flushPendingTitles(now: now) + for tabID in flushed { + refreshAgentEntriesForTitleChange(in: tabID) + } + return flushed + } + /// Single-pane variant for a surface whose own title changed without moving /// the tab title (e.g. an unfocused split's OSC-2 update). func refreshAgentEntryForTitleChange(surfaceID: UUID, in tabId: TerminalTabID) { diff --git a/supacodeTests/TabTitleFlushRefreshTests.swift b/supacodeTests/TabTitleFlushRefreshTests.swift new file mode 100644 index 00000000..f41fba4d --- /dev/null +++ b/supacodeTests/TabTitleFlushRefreshTests.swift @@ -0,0 +1,99 @@ +import AppKit +import Foundation +import GhosttyKit +import Testing + +@testable import supacode + +/// A tab title held back by coalescing must refresh the Active Agents subtitle the +/// same way a title written straight through `updateTitle` does. Those are two +/// separate call sites into `refreshAgentEntriesForTitleChange`, and nothing but a +/// test keeps them from drifting — the withheld title is invisible until the flush +/// lands it, so a flush that forgot to refresh would show a stale subtitle +/// indefinitely rather than for one poll. +@MainActor +struct TabTitleFlushRefreshTests { + private let start = Date(timeIntervalSince1970: 1_000) + + @Test func flushingAWithheldTitleRefreshesTheAgentEntry() { + let fixture = makeFixture() + var received: [ActiveAgentEntry] = [] + fixture.state.onAgentEntryChanged = { received.append($0) } + + #expect(fixture.state.tabManager.updateTitle(fixture.tabId, title: "⠋ building", now: start)) + received.removeAll() + + // Inside the interval, so the tab bar never sees it and no refresh fires yet. + #expect( + fixture.state.tabManager.updateTitle( + fixture.tabId, title: "⠙ building", now: start.addingTimeInterval(0.2)) == false + ) + #expect(received.isEmpty, "A withheld title must not reach the entry either") + + let flushed = fixture.state.flushCoalescedTabTitles(now: start.addingTimeInterval(1.5)) + + #expect(flushed == [fixture.tabId]) + #expect(received.count == 1, "The flush must refresh the entry, not just the tab") + #expect(received.last?.paneTitle == "⠙ building") + } + + /// The flush is called on every detection tick, so the common case — nothing held + /// back — must not emit. Otherwise the coalescing would trade one source of entry + /// churn for another. + @Test func flushingWithNothingWithheldEmitsNothing() { + let fixture = makeFixture() + _ = fixture.state.tabManager.updateTitle(fixture.tabId, title: "⠋ building", now: start) + var received: [ActiveAgentEntry] = [] + fixture.state.onAgentEntryChanged = { received.append($0) } + + #expect(fixture.state.flushCoalescedTabTitles(now: start.addingTimeInterval(5)).isEmpty) + #expect(received.isEmpty) + } + + /// A flush inside the interval must leave the title held, so the one-second spacing + /// is not quietly bypassed by the poll running more often than that. + @Test func aFlushInsideTheIntervalHoldsTheTitle() { + let fixture = makeFixture() + _ = fixture.state.tabManager.updateTitle(fixture.tabId, title: "⠋ building", now: start) + _ = fixture.state.tabManager.updateTitle(fixture.tabId, title: "⠙ building", now: start.addingTimeInterval(0.2)) + var received: [ActiveAgentEntry] = [] + fixture.state.onAgentEntryChanged = { received.append($0) } + + #expect(fixture.state.flushCoalescedTabTitles(now: start.addingTimeInterval(0.5)).isEmpty) + #expect(received.isEmpty) + #expect(fixture.state.tabManager.tabs.first?.title == "⠋ building") + } + + private struct Fixture { + let state: WorktreeTerminalState + let tabId: TerminalTabID + let pane: GhosttySurfaceView + } + + private func makeFixture() -> Fixture { + let state = WorktreeTerminalState( + runtime: GhosttyRuntime(), + worktree: Worktree( + id: "/tmp/repo/worktree", + name: "worktree", + detail: "", + workingDirectory: URL(fileURLWithPath: "/tmp/repo/worktree"), + repositoryRootURL: URL(fileURLWithPath: "/tmp/repo") + ) + ) + let pane = GhosttySurfaceView( + runtime: state.runtime, + workingDirectory: URL(fileURLWithPath: "/tmp/repo/worktree", isDirectory: true), + fontSize: nil, + context: GHOSTTY_SURFACE_CONTEXT_TAB, + skipsSurfaceCreationForTesting: true + ) + let tabId = state.tabManager.createTab(title: "worktree 1", icon: "terminal") + state.surfaces[pane.id] = pane + state.trees[tabId] = SplitTree(view: pane) + state.focusedSurfaceIdByTab[tabId] = pane.id + // An entry is only produced for a pane with a detected agent in a known state. + state.surfaceAgentStates[pane.id] = PaneAgentState(detectedAgent: .claude, state: .working) + return Fixture(state: state, tabId: tabId, pane: pane) + } +} diff --git a/supacodeTests/TerminalTabTitleCoalescingTests.swift b/supacodeTests/TerminalTabTitleCoalescingTests.swift index 0d6a1c94..c8eca9f9 100644 --- a/supacodeTests/TerminalTabTitleCoalescingTests.swift +++ b/supacodeTests/TerminalTabTitleCoalescingTests.swift @@ -127,13 +127,56 @@ struct TerminalTabTitleCoalescingTests { #expect(manager.tabs.first(where: { $0.id == id })?.displayTitle == "my tab") } + /// Asserts the bookkeeping itself, not `flushPendingTitles`'s return value: that + /// value is empty for a closed tab whether or not the prune ran, because the flush + /// skips any id missing from `tabs`. Reading the retained ids is the only way to + /// tell a working prune from a leak that grows with every tab ever closed. @Test func closingATabDropsItsCoalescingState() { let (manager, id) = makeManager() _ = manager.updateTitle(id, title: "⠋ working", now: start) _ = manager.updateTitle(id, title: "⠙ working", now: start.addingTimeInterval(0.2)) + #expect(manager.coalescedTabIDsForTesting == [id], "Both a last-write stamp and a pending title are held") + manager.closeTab(id) + #expect(manager.coalescedTabIDsForTesting.isEmpty, "Closing the tab must drop its bookkeeping, not leak it") #expect(manager.flushPendingTitles(now: start.addingTimeInterval(5)).isEmpty) #expect(manager.tabs.isEmpty) } + + /// A surviving tab must keep its state when a sibling closes, so the prune cannot + /// be "fixed" by clearing everything. + @Test func closingOneTabKeepsAnotherTabsCoalescingState() { + let (manager, kept) = makeManager() + let closed = manager.createTab(title: "second", icon: nil) + _ = manager.updateTitle(kept, title: "⠋ kept", now: start) + _ = manager.updateTitle(closed, title: "⠋ closed", now: start) + + manager.closeTab(closed) + + #expect(manager.coalescedTabIDsForTesting == [kept]) + } + + /// Pins the comparison at `TerminalTabManager.swift`'s `<` against the interval. + /// Without a case landing exactly on the boundary, changing it to `<=` — which + /// would withhold a title that has waited the full interval — passes every other + /// test in this file. + @Test func aTitleArrivingExactlyOnTheIntervalIsWritten() { + let (manager, id) = makeManager() + _ = manager.updateTitle(id, title: "⠋ working", now: start) + let boundary = start.addingTimeInterval(TerminalTabManager.liveTitleCoalescingInterval) + + #expect(manager.updateTitle(id, title: "⠙ working", now: boundary)) + #expect(title(of: manager, id) == "⠙ working") + } + + /// The other side of the same boundary: one instant earlier must still be withheld. + @Test func aTitleArrivingJustInsideTheIntervalIsWithheld() { + let (manager, id) = makeManager() + _ = manager.updateTitle(id, title: "⠋ working", now: start) + let justInside = start.addingTimeInterval(TerminalTabManager.liveTitleCoalescingInterval - 0.001) + + #expect(manager.updateTitle(id, title: "⠙ working", now: justInside) == false) + #expect(title(of: manager, id) == "⠋ working") + } } -- 2.51.2 From 1e8933cb89b9815f257c8101e2ee82b4960344dd Mon Sep 17 00:00:00 2001 From: Simon Heimlicher Date: Fri, 24 Jul 2026 15:28:13 +0200 Subject: [PATCH 12/31] Cut agent session resolution CPU by reusing parsed transcript tails Profiling a 20-pane instance showed agent detection accounting for essentially all steady-state CPU (15.9% of a core), concentrated in session fingerprint matching: detectAgentState 15.9% AgentSessionResolver.resolve 13.0% resolveUncached 13.0% bestMatch 11.4% normalize 6.7% transcriptStrings 2.5% filesystem scan 1.5% tailData (I/O) 0.01% The tails sit in the page cache, so the cost was recomputation, not I/O: every pane re-read, re-parsed, and re-normalized the same transcript bytes each time its 5 s session cache expired. Expirations across panes cluster, so several of these ran concurrently and showed up as CPU spikes against a much lower median. Two changes: TranscriptFragmentCache keys parsed, normalized fragments on the transcript's path and modification date, so an unappended file replays its fragments instead of being re-read. Entries not consulted in a round are pruned, which bounds a cache whose keys would otherwise grow with every append. Matching behavior is unchanged: identical fragments produce identical scores. normalize gains an ASCII fast path. Swift Regex escape stripping plus grapheme-level whitespace splitting dominated the remainder; a single byte scan replaces both. Any non-ASCII byte defers to the original implementation, which is retained as normalizeGeneral so Unicode case folding and the full Unicode whitespace set keep exact semantics. Tests assert the two agree over a hand-built corpus and 2000 seeded random ASCII inputs. resolveUncached takes its inputs as a ResolveRequest to stay within the parameter-count limit. --- .../AgentDetection/AgentSessionResolver.swift | 225 +++++++++++++++--- ...gentSessionFingerprintNormalizeTests.swift | 97 ++++++++ .../TranscriptFragmentCacheTests.swift | 147 ++++++++++++ 3 files changed, 439 insertions(+), 30 deletions(-) create mode 100644 supacodeTests/AgentSessionFingerprintNormalizeTests.swift create mode 100644 supacodeTests/TranscriptFragmentCacheTests.swift diff --git a/supacode/Infrastructure/AgentDetection/AgentSessionResolver.swift b/supacode/Infrastructure/AgentDetection/AgentSessionResolver.swift index c5908658..68884831 100644 --- a/supacode/Infrastructure/AgentDetection/AgentSessionResolver.swift +++ b/supacode/Infrastructure/AgentDetection/AgentSessionResolver.swift @@ -70,6 +70,46 @@ nonisolated struct AgentSessionResolution: Sendable { let isFresh: Bool } +/// Parsed, normalized transcript fragments reused across resolver polls. +/// +/// Fingerprint matching re-reads the same transcript tails every time a pane's +/// session cache expires (5 s while a session stays resolved). The bytes are +/// almost always identical and the tails sit in the page cache, so the cost is +/// re-parsing JSON and re-normalizing text rather than I/O. Keying the parsed +/// result on the file's modification time replays it for unchanged transcripts +/// without altering which candidate wins. +nonisolated struct TranscriptFragmentCache: Sendable { + struct Key: Hashable, Sendable { + let path: String + let modifiedAt: Date + } + + private var entries: [Key: [String]] = [:] + private var consulted: Set = [] + + var count: Int { entries.count } + + /// Returns the cached fragments for `key`, otherwise stores and returns + /// `load()`. A nil `load()` is deliberately not cached: an unreadable tail is + /// transient, and caching the failure would keep a recovered file excluded. + mutating func fragments(for key: Key, load: () -> [String]?) -> [String]? { + consulted.insert(key) + if let cached = entries[key] { return cached } + guard let loaded = load() else { return nil } + entries[key] = loaded + return loaded + } + + /// Drops every entry not consulted since the previous prune: superseded + /// versions of an appended transcript, and files that left the candidate set. + /// Because the key carries a modification date, an actively written transcript + /// mints a new entry per poll; without this the cache would grow without bound. + mutating func pruneUnconsulted() { + entries = entries.filter { consulted.contains($0.key) } + consulted.removeAll(keepingCapacity: true) + } +} + /// Compatibility shim over the per-agent profiles; the actual rules live in /// `AgentSessionProfile`. nonisolated enum AgentSessionPathParser { @@ -94,6 +134,20 @@ actor AgentSessionResolver { var provisionalSoleID: String? } + /// The immutable inputs of one uncached resolution, grouped so the scan and + /// match steps take a single subject rather than a long parameter list. + private struct ResolveRequest { + let identified: IdentifiedAgentProcess + let processStartedAt: Date + let workingDirectory: URL? + let activeText: String + /// A relocated agent config root, when the runtime supports one. Selects the + /// rooted path parser and candidate roots, and suppresses the pid-keyed + /// lookup, whose artifacts only ever live in the default home. + let configRoot: URL? + let now: Date + } + /// Unresolved lookups retry quickly while the narrow scan stays cheap, then /// back off exponentially while the pane stays ambiguous; wide fallback /// scans (full history trees) start at the slow end. 15 s cap keeps a @@ -132,6 +186,9 @@ actor AgentSessionResolver { } private var cache: [CacheKey: CachedResult] = [:] + /// Per-pane transcript fragment reuse, kept beside `cache` so both are evicted + /// on the same liveness check. + private var fragmentCaches: [CacheKey: TranscriptFragmentCache] = [:] private let fileManager: FileManager private let homeDirectory: URL @@ -169,14 +226,20 @@ actor AgentSessionResolver { } } + var fragments = fragmentCaches[key] ?? TranscriptFragmentCache() let (resolved, usedWideScan) = resolveUncached( - identified: identified, - processStartedAt: startedAt, - workingDirectory: workingDirectory, - activeText: activeText, - configRoot: configRoot, - now: now + ResolveRequest( + identified: identified, + processStartedAt: startedAt, + workingDirectory: workingDirectory, + activeText: activeText, + configRoot: configRoot, + now: now + ), + fragments: &fragments ) + fragments.pruneUnconsulted() + fragmentCaches[key] = fragments var session = resolved var provisionalID: String? if let candidate = session, candidate.confidence == .medium { @@ -202,6 +265,7 @@ actor AgentSessionResolver { cache = cache.filter { entry in ProcessDetection.processStartDate(pid: entry.key.pid) == entry.key.startedAt } + fragmentCaches = fragmentCaches.filter { cache[$0.key] != nil } } return AgentSessionResolution(session: session, isFresh: true) } @@ -229,15 +293,12 @@ actor AgentSessionResolver { } private func resolveUncached( - identified: IdentifiedAgentProcess, - processStartedAt: Date, - workingDirectory: URL?, - activeText: String, - configRoot: URL? = nil, - now: Date + _ request: ResolveRequest, + fragments cache: inout TranscriptFragmentCache ) -> (session: AgentSession?, usedWideScan: Bool) { + let identified = request.identified let profile = AgentSessionProfile.profile(for: identified.agent) - let parsePath = Self.pathParser(profile: profile, configRoot: configRoot) + let parsePath = Self.pathParser(profile: profile, configRoot: request.configRoot) let openSessions = ProcessDetection.openFilePaths(pid: identified.process.pid) .compactMap { parsePath($0) } @@ -261,8 +322,8 @@ actor AgentSessionResolver { // pid-keyed artifacts live in the default home; a relocated config root // has no equivalent (no bound-capable runtime defines one). - if configRoot == nil, - let session = profile.pidKeyedSession?(homeDirectory, identified.process.pid, processStartedAt) + if request.configRoot == nil, + let session = profile.pidKeyedSession?(homeDirectory, identified.process.pid, request.processStartedAt) { return (session, false) } @@ -275,8 +336,9 @@ actor AgentSessionResolver { return AgentSessionCandidate(session: session, modifiedAt: modifiedAt) } if let matched = AgentSessionFingerprintMatcher.bestMatch( - activeText: activeText, - candidates: openCandidates + activeText: request.activeText, + candidates: openCandidates, + fragments: &cache ) { let resolved = AgentSession( id: matched.session.id, @@ -289,12 +351,16 @@ actor AgentSessionResolver { let (candidates, usedWideScan) = recentCandidates( profile: profile, - processStartedAt: processStartedAt, - workingDirectory: workingDirectory, - configRoot: configRoot, - now: now + processStartedAt: request.processStartedAt, + workingDirectory: request.workingDirectory, + configRoot: request.configRoot, + now: request.now ) - if let matched = AgentSessionFingerprintMatcher.bestMatch(activeText: activeText, candidates: candidates) { + if let matched = AgentSessionFingerprintMatcher.bestMatch( + activeText: request.activeText, + candidates: candidates, + fragments: &cache + ) { let resolved = AgentSession( id: matched.session.id, transcriptPath: matched.session.transcriptPath, @@ -303,7 +369,8 @@ actor AgentSessionResolver { ) return (resolved, usedWideScan) } - let sole = AgentSessionCandidate.uniqueActiveCandidate(candidates, processStartedAt: processStartedAt)?.session + let sole = AgentSessionCandidate.uniqueActiveCandidate(candidates, processStartedAt: request.processStartedAt)? + .session return (sole, usedWideScan) } @@ -478,9 +545,19 @@ actor AgentSessionResolver { } nonisolated enum AgentSessionFingerprintMatcher { + /// Convenience for call sites with no cache to reuse (tests, one-shot lookups). static func bestMatch( activeText: String, candidates: [AgentSessionCandidate] + ) -> AgentSessionCandidate? { + var scratch = TranscriptFragmentCache() + return bestMatch(activeText: activeText, candidates: candidates, fragments: &scratch) + } + + static func bestMatch( + activeText: String, + candidates: [AgentSessionCandidate], + fragments cache: inout TranscriptFragmentCache ) -> AgentSessionCandidate? { let screen = normalize(activeText) guard screen.count >= 12 else { return nil } @@ -494,16 +571,12 @@ nonisolated enum AgentSessionFingerprintMatcher { for group in bySession.values { var sessionScoreable = false for candidate in group.sorted(by: { $0.modifiedAt > $1.modifiedAt }).prefix(2) { - guard let path = candidate.session.transcriptPath, let data = tailData(at: path) else { continue } - // Lossy decoding is deliberate: the tail window can start mid-character - // in a multi-byte transcript, and a failable conversion would void the - // whole tail instead of just the cut first line. - // swiftlint:disable:next optional_data_string_conversion - let fragments = transcriptStrings(String(decoding: data, as: UTF8.self)) // Scoreable means the session produced at least one fragment long // enough to actually enter the comparison — fragments below the floor // ("OK") are no testimony at all. - let comparable = fragments.map(normalize).filter { $0.count >= 12 } + guard let path = candidate.session.transcriptPath, + let comparable = comparableFragments(at: path, modifiedAt: candidate.modifiedAt, cache: &cache) + else { continue } if !comparable.isEmpty { sessionScoreable = true } let score = comparable.reduce(0) { best, normalized in if screen.contains(normalized) { return max(best, min(200, normalized.count + 80)) } @@ -532,7 +605,41 @@ nonisolated enum AgentSessionFingerprintMatcher { return winner.best.0 } + /// Normalized, length-filtered fragments for one transcript tail, served from + /// `cache` whenever the file has not been appended to since the last poll. + private static func comparableFragments( + at path: URL, + modifiedAt: Date, + cache: inout TranscriptFragmentCache + ) -> [String]? { + cache.fragments(for: TranscriptFragmentCache.Key(path: path.path, modifiedAt: modifiedAt)) { + guard let data = tailData(at: path) else { return nil } + // Lossy decoding is deliberate: the tail window can start mid-character + // in a multi-byte transcript, and a failable conversion would void the + // whole tail instead of just the cut first line. + // swiftlint:disable:next optional_data_string_conversion + return transcriptStrings(String(decoding: data, as: UTF8.self)) + .map(normalize) + .filter { $0.count >= 12 } + } + } + + /// Strips ANSI escapes, folds case, and collapses whitespace runs so screen + /// text and transcript text compare on content alone. + /// + /// The ASCII fast path exists because this runs over every transcript fragment + /// of every fingerprint match and dominated the resolver's CPU: Swift Regex + /// and grapheme-level whitespace splitting are both far more expensive than a + /// single byte scan. Any non-ASCII byte defers to `normalizeGeneral`, so + /// Unicode case folding and the full Unicode whitespace set keep their exact + /// semantics rather than being approximated. static func normalize(_ value: String) -> String { + normalizeASCII(value) ?? normalizeGeneral(value) + } + + /// Reference implementation. `normalize` must agree with it for every input; + /// `AgentSessionFingerprintNormalizeTests` asserts that over a corpus. + static func normalizeGeneral(_ value: String) -> String { value .replacing(#/\u{001B}\[[0-?]*[ -\/]*[@-~]/#, with: " ") .lowercased() @@ -540,6 +647,64 @@ nonisolated enum AgentSessionFingerprintMatcher { .joined(separator: " ") } + /// Returns nil when `value` holds any non-ASCII byte, leaving those inputs to + /// the general path. + private static func normalizeASCII(_ value: String) -> String? { + let utf8 = value.utf8 + // Built as scalars rather than bytes: every byte kept here is ASCII, so the + // conversion is exact and needs no decoding pass over the result. + var output = String.UnicodeScalarView() + output.reserveCapacity(utf8.count) + var pendingSeparator = false + var index = utf8.startIndex + + while index < utf8.endIndex { + let byte = utf8[index] + guard byte < 0x80 else { return nil } + // A well-formed CSI sequence becomes a space, which is itself a separator; + // a bare ESC matches no sequence and survives as ordinary text. + if byte == 0x1B, let end = csiEnd(utf8, from: index) { + pendingSeparator = true + index = end + continue + } + if isASCIIWhitespace(byte) { + pendingSeparator = true + } else { + // Leading whitespace produces no separator because nothing precedes it, + // and a trailing run is simply never flushed — matching the trim that + // `split` + `joined` performs. + if pendingSeparator { + if !output.isEmpty { output.append(Unicode.Scalar(UInt8(0x20))) } + pendingSeparator = false + } + output.append(Unicode.Scalar(byte >= 0x41 && byte <= 0x5A ? byte + 0x20 : byte)) + } + index = utf8.index(after: index) + } + return String(output) + } + + /// The Unicode `White_Space` members that fall in the ASCII range. + private static func isASCIIWhitespace(_ byte: UInt8) -> Bool { + byte == 0x20 || (0x09...0x0D).contains(byte) + } + + /// Index just past a CSI sequence starting at `start`, or nil if the bytes + /// there do not form one: `ESC [` , parameters, intermediates, then a final byte. + private static func csiEnd( + _ utf8: String.UTF8View, + from start: String.UTF8View.Index + ) -> String.UTF8View.Index? { + var index = utf8.index(after: start) + guard index < utf8.endIndex, utf8[index] == 0x5B else { return nil } + index = utf8.index(after: index) + while index < utf8.endIndex, (0x30...0x3F).contains(utf8[index]) { index = utf8.index(after: index) } + while index < utf8.endIndex, (0x20...0x2F).contains(utf8[index]) { index = utf8.index(after: index) } + guard index < utf8.endIndex, (0x40...0x7E).contains(utf8[index]) else { return nil } + return utf8.index(after: index) + } + private static func tailData(at url: URL, byteLimit: UInt64 = 131_072) -> Data? { guard let handle = try? FileHandle(forReadingFrom: url) else { return nil } defer { try? handle.close() } diff --git a/supacodeTests/AgentSessionFingerprintNormalizeTests.swift b/supacodeTests/AgentSessionFingerprintNormalizeTests.swift new file mode 100644 index 00000000..c87ca26a --- /dev/null +++ b/supacodeTests/AgentSessionFingerprintNormalizeTests.swift @@ -0,0 +1,97 @@ +import Foundation +import Testing + +@testable import supacode + +/// `normalize` runs an ASCII byte-scan fast path in place of the Swift Regex + +/// grapheme-split reference. It must be indistinguishable from that reference +/// for every input, or fingerprint matching would silently change which +/// transcript wins. +struct AgentSessionFingerprintNormalizeTests { + /// Inputs chosen to exercise every branch of the fast path and every reason it + /// bails out to the general path. + private static let corpus: [String] = [ + "", + " ", + "\t\n \r\n", + "plain text", + " leading and trailing ", + "MiXeD CaSe TEXT", + "collapse\t\tmultiple\n\nwhitespace\u{000B}runs\u{000C}here", + "windows\r\nline\r\nendings", + // ANSI: reset, params, private-mode param byte, an intermediate byte. + "\u{001B}[0mreset", + "\u{001B}[1;31mred\u{001B}[0m normal", + "\u{001B}[?25hcursor shown", + "\u{001B}[ qbar cursor", + "adjacent\u{001B}[0m\u{001B}[1msequences", + "\u{001B}[2Jleading sequence", + "trailing sequence\u{001B}[0m", + // Malformed: these must survive as ordinary text, not be swallowed. + "bare \u{001B} escape", + "truncated \u{001B}[", + "no final byte \u{001B}[1;2", + "escape at end \u{001B}", + "\u{001B}", + "\u{001B}[", + // Non-ASCII must defer to the general path. + "héllo wörld", + "CAFÉ AU LAIT", + "日本語のテキストです", + "emoji 🎉 mixed with ascii", + "non-breaking\u{00A0}space", + "next\u{0085}line", + "İstanbul uppercase dotted i", + "fi ligature", + "\u{001B}[31mré\u{001B}[0m d", + "mixed ascii and ünicode with spacing", + ] + + @Test func fastPathMatchesReferenceForEveryCorpusInput() { + for input in Self.corpus { + #expect( + AgentSessionFingerprintMatcher.normalize(input) + == AgentSessionFingerprintMatcher.normalizeGeneral(input), + "normalize diverged from the reference for \(String(reflecting: input))" + ) + } + } + + @Test func fastPathMatchesReferenceForRandomASCII() { + // Random byte soup over the interesting ASCII range catches sequence shapes + // the hand-written corpus does not enumerate. + var generator = SeededGenerator(seed: 0x5EED_1234) + let alphabet: [Character] = Array("abzAZ019 \t\n\r\u{000B}\u{000C}[;?@~\u{001B}mJq/ ") + for _ in 0..<2000 { + let length = Int.random(in: 0...40, using: &generator) + let input = String((0.. UInt64 { + state ^= state << 13 + state ^= state >> 7 + state ^= state << 17 + return state + } +} diff --git a/supacodeTests/TranscriptFragmentCacheTests.swift b/supacodeTests/TranscriptFragmentCacheTests.swift new file mode 100644 index 00000000..28dc3f73 --- /dev/null +++ b/supacodeTests/TranscriptFragmentCacheTests.swift @@ -0,0 +1,147 @@ +import Foundation +import Testing + +@testable import supacode + +/// Fingerprint matching re-reads the same transcript tails every time a pane's +/// session cache expires. `TranscriptFragmentCache` replays the parsed result +/// for unchanged files; these tests pin both the reuse and the bounding. +struct TranscriptFragmentCacheTests { + private func key(_ path: String, _ secondsSinceEpoch: TimeInterval) -> TranscriptFragmentCache.Key { + TranscriptFragmentCache.Key(path: path, modifiedAt: Date(timeIntervalSince1970: secondsSinceEpoch)) + } + + @Test func reusesFragmentsForAnUnchangedFile() { + var cache = TranscriptFragmentCache() + var loads = 0 + let target = key("/tmp/a.jsonl", 100) + + let first = cache.fragments(for: target) { + loads += 1 + return ["parsed fragment"] + } + let second = cache.fragments(for: target) { + loads += 1 + return ["should not be reached"] + } + + #expect(loads == 1) + #expect(first == ["parsed fragment"]) + #expect(second == ["parsed fragment"]) + } + + @Test func reloadsWhenTheFileIsAppendedTo() { + var cache = TranscriptFragmentCache() + var loads = 0 + + _ = cache.fragments(for: key("/tmp/a.jsonl", 100)) { + loads += 1 + return ["old"] + } + let updated = cache.fragments(for: key("/tmp/a.jsonl", 101)) { + loads += 1 + return ["new"] + } + + #expect(loads == 2) + #expect(updated == ["new"]) + } + + @Test func doesNotCacheAnUnreadableTail() { + var cache = TranscriptFragmentCache() + var loads = 0 + let target = key("/tmp/gone.jsonl", 100) + + let missing = cache.fragments(for: target) { + loads += 1 + return nil + } + let recovered = cache.fragments(for: target) { + loads += 1 + return ["now readable"] + } + + // Caching the failure would keep a briefly unreadable transcript excluded + // from every later match. + #expect(loads == 2) + #expect(missing == nil) + #expect(recovered == ["now readable"]) + } + + @Test func pruneDropsOnlyEntriesNotConsultedSinceTheLastPrune() { + var cache = TranscriptFragmentCache() + let stable = key("/tmp/stable.jsonl", 100) + let superseded = key("/tmp/busy.jsonl", 100) + _ = cache.fragments(for: stable) { ["stable"] } + _ = cache.fragments(for: superseded) { ["v1"] } + #expect(cache.count == 2) + + cache.pruneUnconsulted() + #expect(cache.count == 2, "Both were consulted in this round, so both survive") + + // Next round: the busy transcript is appended to, so its old key is never + // consulted again and must not accumulate. + _ = cache.fragments(for: stable) { ["stable"] } + _ = cache.fragments(for: key("/tmp/busy.jsonl", 101)) { ["v2"] } + cache.pruneUnconsulted() + + #expect(cache.count == 2) + var loads = 0 + _ = cache.fragments(for: superseded) { + loads += 1 + return ["v1 again"] + } + #expect(loads == 1, "The superseded entry was evicted, so it must reload") + } + + @Test func bestMatchServesASecondCallFromTheCache() throws { + let root = FileManager.default.temporaryDirectory + .appending(path: "prowl-fragment-cache-\(UUID().uuidString)", directoryHint: .isDirectory) + try FileManager.default.createDirectory(at: root, withIntermediateDirectories: true) + defer { try? FileManager.default.removeItem(at: root) } + + let firstURL = root.appending(path: "first.jsonl") + let secondURL = root.appending(path: "second.jsonl") + try #"{"type":"user","message":{"content":"Refactor authentication middleware without changing its API."}}"# + .write(to: firstURL, atomically: true, encoding: .utf8) + try #"{"type":"user","message":{"content":"Investigate the unrelated rendering regression."}}"# + .write(to: secondURL, atomically: true, encoding: .utf8) + + let modifiedAt = Date(timeIntervalSince1970: 1_000) + let candidates = [ + AgentSessionCandidate( + session: AgentSession(id: "first", transcriptPath: firstURL, source: .recentFile), + modifiedAt: modifiedAt + ), + AgentSessionCandidate( + session: AgentSession(id: "second", transcriptPath: secondURL, source: .recentFile), + modifiedAt: modifiedAt + ), + ] + let activeText = "❯ Refactor authentication middleware without changing its API." + + var cache = TranscriptFragmentCache() + let first = AgentSessionFingerprintMatcher.bestMatch( + activeText: activeText, + candidates: candidates, + fragments: &cache + ) + #expect(first?.session.id == "first") + + // Deleting the transcripts makes a re-read impossible, so a second match on + // the same keys can only succeed by replaying the cached fragments. + try FileManager.default.removeItem(at: firstURL) + try FileManager.default.removeItem(at: secondURL) + + let cached = AgentSessionFingerprintMatcher.bestMatch( + activeText: activeText, + candidates: candidates, + fragments: &cache + ) + #expect(cached?.session.id == "first") + + // A cacheless call over the same candidates now has nothing to read, which + // confirms the previous call really was served from the cache. + #expect(AgentSessionFingerprintMatcher.bestMatch(activeText: activeText, candidates: candidates) == nil) + } +} -- 2.51.2 From c97cbb4db9a3fa98d5dd301e9c55d62836dfd983 Mon Sep 17 00:00:00 2001 From: Simon Heimlicher Date: Fri, 24 Jul 2026 16:23:31 +0200 Subject: [PATCH 13/31] Skip escape stripping when a fragment holds no escape byte Component timing over 12 real transcript tails showed the escape-stripping regex costing 49.9 ms against 10.4 ms for case folding, while 238 of 240 fragments contained no ESC byte at all. A pattern anchored on ESC cannot match a string without one, so proving absence with a byte scan (11.5 ms) short-circuits the regex entirely on almost every fragment. Unlike the ASCII fast path this applies to all input, and it is the larger of the two wins: original 101.6 ms/round 1.00x + escape-absence guard 64.1 ms/round 1.58x + ASCII fast path 57.9 ms/round 1.75x The normalize tests now compare both shipped paths against a pristine copy of the original formulation rather than against each other, so neither optimization can drift from the semantics the matcher was built on. --- .../AgentDetection/AgentSessionResolver.swift | 12 +++++-- ...gentSessionFingerprintNormalizeTests.swift | 32 +++++++++++++++---- 2 files changed, 35 insertions(+), 9 deletions(-) diff --git a/supacode/Infrastructure/AgentDetection/AgentSessionResolver.swift b/supacode/Infrastructure/AgentDetection/AgentSessionResolver.swift index 68884831..a4caf94e 100644 --- a/supacode/Infrastructure/AgentDetection/AgentSessionResolver.swift +++ b/supacode/Infrastructure/AgentDetection/AgentSessionResolver.swift @@ -640,8 +640,16 @@ nonisolated enum AgentSessionFingerprintMatcher { /// Reference implementation. `normalize` must agree with it for every input; /// `AgentSessionFingerprintNormalizeTests` asserts that over a corpus. static func normalizeGeneral(_ value: String) -> String { - value - .replacing(#/\u{001B}\[[0-?]*[ -\/]*[@-~]/#, with: " ") + // A pattern anchored on ESC cannot match a string with no ESC byte, and + // nearly every transcript fragment has none. Proving absence with a byte + // scan is several times cheaper than letting the regex engine walk the + // whole string to reach the same conclusion. + let stripped = + value.utf8.contains(0x1B) + ? value.replacing(#/\u{001B}\[[0-?]*[ -\/]*[@-~]/#, with: " ") + : value + return + stripped .lowercased() .split(whereSeparator: \Character.isWhitespace) .joined(separator: " ") diff --git a/supacodeTests/AgentSessionFingerprintNormalizeTests.swift b/supacodeTests/AgentSessionFingerprintNormalizeTests.swift index c87ca26a..c8ee1462 100644 --- a/supacodeTests/AgentSessionFingerprintNormalizeTests.swift +++ b/supacodeTests/AgentSessionFingerprintNormalizeTests.swift @@ -47,12 +47,26 @@ struct AgentSessionFingerprintNormalizeTests { "mixed ascii and ünicode with spacing", ] - @Test func fastPathMatchesReferenceForEveryCorpusInput() { + /// The original formulation, before either the ASCII fast path or the + /// escape-absence guard. Both shipped paths must reproduce it exactly. + private static func pristine(_ value: String) -> String { + value + .replacing(#/\u{001B}\[[0-?]*[ -\/]*[@-~]/#, with: " ") + .lowercased() + .split(whereSeparator: \Character.isWhitespace) + .joined(separator: " ") + } + + @Test func bothPathsMatchTheOriginalForEveryCorpusInput() { for input in Self.corpus { + let expected = Self.pristine(input) + #expect( + AgentSessionFingerprintMatcher.normalize(input) == expected, + "normalize diverged for \(String(reflecting: input))" + ) #expect( - AgentSessionFingerprintMatcher.normalize(input) - == AgentSessionFingerprintMatcher.normalizeGeneral(input), - "normalize diverged from the reference for \(String(reflecting: input))" + AgentSessionFingerprintMatcher.normalizeGeneral(input) == expected, + "normalizeGeneral diverged for \(String(reflecting: input))" ) } } @@ -65,10 +79,14 @@ struct AgentSessionFingerprintNormalizeTests { for _ in 0..<2000 { let length = Int.random(in: 0...40, using: &generator) let input = String((0.. Date: Sun, 2 Aug 2026 00:28:12 +0900 Subject: [PATCH 14/31] Make untracked line counts cached and honest --- .../000-plan.md | 158 ++++++++++++++++ .../001-action.md | 105 +++++++++++ docs-ai/README.md | 1 + docs/components/diff-view.md | 7 + supacode/Clients/Git/GitClient.swift | 177 ++++++++++++++---- supacode/Clients/Git/GitClientTypes.swift | 25 +++ .../Clients/Git/UntrackedLineCountCache.swift | 108 +++++++++++ .../Repositories/GitClientDependency.swift | 2 +- .../Domain/LineChangeBadgePresentation.swift | 29 +++ supacode/Domain/WorktreeInfoEntry.swift | 15 +- .../RepositoriesFeature+CoreReducer.swift | 13 +- ...epositoriesFeature+WorkspaceChildren.swift | 10 +- .../RepositoriesFeature+WorktreeState.swift | 43 ++--- .../Reducer/RepositoriesFeature.swift | 7 +- .../Repositories/Views/WorktreeRow.swift | 41 ++-- supacode/Support/CustomDump+Extensions.swift | 1 + supacodeTests/GitClientLineChangesTests.swift | 170 ++++++++++++++--- .../LineChangeBadgePresentationTests.swift | 51 +++++ supacodeTests/RepositoriesFeatureTests.swift | 105 ++++++++--- 19 files changed, 919 insertions(+), 149 deletions(-) create mode 100644 docs-ai/056-performance-optimization-2026-08/000-plan.md create mode 100644 docs-ai/056-performance-optimization-2026-08/001-action.md create mode 100644 supacode/Clients/Git/UntrackedLineCountCache.swift create mode 100644 supacode/Domain/LineChangeBadgePresentation.swift create mode 100644 supacodeTests/LineChangeBadgePresentationTests.swift diff --git a/docs-ai/056-performance-optimization-2026-08/000-plan.md b/docs-ai/056-performance-optimization-2026-08/000-plan.md new file mode 100644 index 00000000..b962c021 --- /dev/null +++ b/docs-ai/056-performance-optimization-2026-08/000-plan.md @@ -0,0 +1,158 @@ +# 056 — Performance Optimization 2026-08: Plan + +| | | +| --- | --- | +| **Status** | Implemented | +| **Anchor date** | 2026-08-01 | +| **Primary PRs** | #644 and its fork follow-up; review queue #645–#650 | +| **Related** | [032-performance-hardening](../032-performance-hardening/000-plan.md), [037-line-diff-tracking](../037-line-diff-tracking/000-plan.md), `docs/components/diff-view.md` | + +## Background + +The August 2026 performance series started with seven independently reviewable pull +requests, #644–#650. Each reports a sampled hot path from a many-pane Prowl workload and +proposes a focused optimization. The changes span untracked-line counting, Debug-only TCA +logging, agent detection and session matching, sidebar invalidation, worktree lookup, and +animated terminal titles. + +This entry records the cross-cutting standard used to review and integrate that series: + +- reproduce or independently measure the claimed cost where practical; +- preserve user-visible and CLI semantics unless an explicit product decision says + otherwise; +- treat cache identity, invalidation, lifetime, concurrency, and bounded growth as part of + correctness; +- require tests that can fail when the optimized path stops being equivalent; +- keep each PR independently reviewable instead of combining unrelated hot paths. + +The first application is #644. Its `memchr` scan removes the dominant per-byte +`Data.Iterator` overhead, but its per-file 2 MiB cutoff silently maps a present, readable +untracked text file to zero added lines. If that is the only change, the worktree badge +disappears even though Show Diff still includes the file. + +## Measured baseline for #644 + +An independent Release-optimized benchmark on an Apple M2 Pro mirrored the production +64 KiB reader and 8 KiB binary probe. Warm-cache medians were: + +| Input | Original `Data.reduce` | `memchr` | Incremental gain from skipping after `memchr` | +| --- | ---: | ---: | ---: | +| 2 MiB, sample-like text | 8.50 ms | 0.36 ms | 0.36 ms | +| 2 MiB, source-like text | 8.58 ms | 0.54 ms | 0.54 ms | +| 35 MiB, sample-like text | 148–151 ms | 7–12 ms | about 9 ms | +| 66 MiB, sample-like text | 279–289 ms | 22–23 ms | about 23 ms | + +The scan optimization therefore removes roughly 94–96% of the normal 2 MiB CPU cost +without changing behavior. The hard cutoff saves additional reads for much larger files, +but 2 MiB is not a meaningful performance cliff and is too low to justify silently losing +the exact badge contract. + +The benchmark also found a density tail: repeated `memchr` calls are fastest for normal +text, but a 2 MiB all-newline buffer took about 16.7 ms versus 8.6 ms for `Data.reduce` and +1.1 ms for a raw-pointer loop. The shipped scanner should retain the sparse-text win while +bounding this dense-match case. + +## Goals + +- Preserve #644's original author commits and measurable scan improvement. +- Remove the silent per-file 2 MiB semantic cutoff. +- Cache exact untracked-file line counts across refreshes using stable file metadata, so an + unchanged large capture is not reread when another file triggers FSEvents. +- Bound uncached work with one deterministic byte budget for the whole refresh, not a + per-file limit that can still scan an unbounded aggregate. +- Propagate incomplete-count state to the sidebar and workspace-child rows instead of + coalescing omitted files to zero. +- Keep the diff badge visible and clickable when omitted untracked files are the only + changes. +- Give VoiceOver and the tooltip a truthful explanation of incomplete addition counts. +- Retain exact tracked additions/deletions and the existing binary probe behavior. + +### Non-goals + +- No user-facing setting for byte budgets or cache policy in this wave. +- No replacement of `git diff HEAD --shortstat` or the FSEvents scheduling pipeline. +- No combined implementation of #645–#650; each remains an independent review and merge + decision. +- No claim that author-reported sampling numbers are independently verified until that PR + receives its own review. + +## Design / Approach + +### Exact cache + +Introduce a long-lived, concurrency-safe untracked-line cache shared by live `GitClient` +instances. Entries are scoped by worktree and relative path and fingerprinted with file +identity, byte size, and modification date. An unchanged fingerprint reuses either the +exact text line count or the binary-file result. Every current `git ls-files --others` +result prunes disappeared paths from that worktree's cache. + +Cache misses are scanned outside cache isolation so independent worktrees are not forced +through one serialized I/O executor. Updates are committed with their fingerprint; a +concurrent stale result cannot be reused after the file metadata changes. + +### Aggregate scan budget + +Apply a 32 MiB budget to cache misses for one `lineChanges` refresh. Cached files consume no +budget. Sort misses by size so the budget yields exact counts for the largest useful set of +files. Files that do not fit remain omitted and increment an explicit omission count. + +The scanner also enforces the remaining budget while reading, covering a file that grows +after metadata collection. A single per-file 2 MiB threshold is removed: besides hiding +legitimate generated source, it does not bound 100 files of 1.9 MiB each. + +### Honest presentation + +Carry a structured line-change result with exact counted additions, exact removals, and the +number of omitted untracked files. The sidebar renders exact counts as today. When additions +are incomplete, it renders `+N…`; if no additions were counted, it renders `+…`. The tooltip +and accessibility label state that the addition count is incomplete and how many untracked +files were not counted. The badge remains a Show Diff button. + +### Dense-match scan + +Keep `memchr` for normal sparse text. After a bounded number of matches in one chunk, scan +the remainder through contiguous raw bytes, avoiding one C call per byte for newline-dense +input while preserving exact counts. + +## Performance PR review queue + +The descriptions and heads below were confirmed on 2026-08-01. All claims remain pending +independent code-path and test review except #644, which is the active implementation wave. + +| PR | Confirmed scope | Review focus / placeholder | Head | +| --- | --- | --- | --- | +| #644 | Replace `Data.Iterator` line scans and avoid repeated large untracked-file work | Active: preserve exact badge semantics with cache, aggregate budget, and explicit incomplete state | `978b7b59` | +| #645 | Gate Debug TCA action reflection/state-diff logging behind `PROWL_LOG_TCA_ACTIONS` | Verify flag parsing, logging contract, Debug-only scope, and pass-through reducer semantics | `616bbf4b` | +| #646 | Memoize per-surface agent screen parsing when agent and visible text are unchanged | Verify cache invalidation, stabilization timing, observation isolation, and surface teardown | `b2ac2936` | +| #647 | Deduplicate raw-state-only agent emissions and narrow sidebar invalidation | Decide whether stale CLI `raw_state` is acceptable; trace UI/CLI ownership before merge | `e77ba660` | +| #648 | Replace per-agent worktree scans/path resolution with a cached directory index | Verify deepest-match and symlink semantics, cache invalidation, render-path purity, and current CI | `08773383` | +| #649 | Coalesce animated terminal-title writes and remove quadratic tab lookup | Verify final-title delivery, close/prune lifecycle, custom/locked titles, and clock boundaries | `b77888f3` | +| #650 | Cache parsed transcript tails and fast-path fingerprint normalization | Verify append/mtime invalidation, cache pruning, Unicode equivalence, collision behavior, and current CI | `c97cbb4` | + +## Alternatives & decisions + +- **Hard per-file cutoff:** rejected as the primary mechanism. It creates a discontinuity at + an arbitrary size, silently changes visible behavior, and does not bound aggregate work. +- **Only document the cutoff:** rejected. Documentation cannot make a false exact number or + a disappeared badge truthful. +- **Scan everything with `memchr`:** acceptable CPU behavior for normal local files, but it + repeats I/O after unrelated FSEvents and remains unbounded for pathological worktrees. +- **Cache without a safety budget:** preserves exactness in steady state but allows an + unbounded first refresh. The aggregate budget keeps the worst case explicit and bounded. +- **Time budget:** rejected for now because it makes tests and visible results depend on + hardware and load. A byte budget is deterministic and measurable. +- **Raw-pointer scan only:** stable across newline density but slower than `memchr` on the + motivating sparse-text capture. A bounded hybrid keeps both properties. + +## Validation + +- Observe new cache, invalidation, aggregate-budget, and incomplete-result tests fail before + implementation, then pass. +- Cover reducer/state transitions where an omitted file is the only worktree change. +- Cover visual/accessibility strings through a pure badge-presentation model. +- Run the complete `GitClientLineChangesTests` and affected repository tests. +- Run `make check` and `make build-app` before publishing. +- Re-run a Release-optimized microbenchmark for normal and newline-dense inputs after the + hybrid scanner change. + +## Amendments diff --git a/docs-ai/056-performance-optimization-2026-08/001-action.md b/docs-ai/056-performance-optimization-2026-08/001-action.md new file mode 100644 index 00000000..818f4ac1 --- /dev/null +++ b/docs-ai/056-performance-optimization-2026-08/001-action.md @@ -0,0 +1,105 @@ +# 056 — Performance Optimization 2026-08: Action Log + +| | | +| --- | --- | +| **Status** | Implemented | +| **Anchor date** | 2026-08-01 | +| **Primary PRs** | #644; follow-up PR from `perf/untracked-line-count-completion` | + +## Outcome + +The follow-up preserves both commits from #644 and keeps its `memchr` speedup, +but removes the silent 2 MiB per-file cutoff. Untracked-file line counts now use +an exact metadata cache plus one 32 MiB budget across uncached files for a +refresh. Cached files cost no scan budget, while files that do not fit are +reported explicitly instead of being folded into an exact zero. + +The sidebar and workspace-child paths carry that incomplete state through the +same `GitLineChanges` value. An incomplete additions badge remains visible and +clickable, renders `+N…` (or `+…` when no additions have been counted yet), and +explains the omission count in its tooltip and accessibility label. Tracked +additions and removals remain exact, and Show Diff continues to list all changed +files. + +## Implementation map + +- `GitClient.swift` — removes the per-file cutoff, applies one deterministic + cache-miss byte budget, orders misses small-first, and returns counted lines + plus the number of omitted files. Its sparse `memchr` scan switches to a raw + pointer loop after 2,048 matches in one 64 KiB chunk to bound newline-dense + input. +- `UntrackedLineCountCache.swift` — stores exact text counts and binary results + by worktree/path and file identity, size, and modification date. It prunes + disappeared paths and retains at most 128 worktree roots with LRU eviction. + File I/O stays outside the short cache lock. +- `GitClientTypes.swift` and the repository reducer/state files — carry one + structured `GitLineChanges` value through regular worktrees and project + workspace children, including the omitted-untracked-file count. +- `LineChangeBadgePresentation.swift` and `WorktreeRow.swift` — keep incomplete + counts visible without presenting a lower bound as exact, including truthful + tooltip and VoiceOver text. +- `docs/components/diff-view.md` — documents the cache, refresh-wide budget, + incomplete label, and unchanged Show Diff behavior. +- `docs-ai/056-performance-optimization-2026-08/000-plan.md` — records the + review standard and placeholders for #645–#650 so each remaining performance + PR can be reviewed independently. + +## Measurements + +An Apple M2 Pro Release build using the production 64 KiB reader showed that +#644's byte scan already removes most of the motivating CPU cost: a normal 2 MiB +text file fell from about 8.5 ms with `Data.reduce` to 0.36–0.54 ms with +`memchr`. Skipping that file saves only the remaining sub-millisecond scan on a +warm cache. Larger sample-like files showed a more material incremental saving: +roughly 9 ms at 35 MiB and 23 ms at 66 MiB. + +A second in-memory benchmark verified the hybrid scanner after implementation: + +| Input | `Data.reduce` | Pure `memchr` | Hybrid | +| --- | ---: | ---: | ---: | +| 2 MiB sample-like text | 7.900 ms | 0.113 ms | 0.118 ms | +| 2 MiB source-like text | 7.845 ms | 0.294 ms | 0.294 ms | +| 2 MiB all-newline text | 7.832 ms | 16.411 ms | 0.507 ms | +| 35 MiB sample-like text | 137.342 ms | 1.974 ms | 2.071 ms | + +The fallback therefore removes the dense-match regression without materially +changing the normal-text result. The 32 MiB aggregate budget is a deterministic +safety bound rather than a claimed performance cliff; the cache is what removes +repeated scans from the steady-state refresh path. + +## Verification + +- The legacy-cutoff test was changed first to require an exact count at 2 MiB; + it failed against #644's guard before implementation. +- Cache reuse, invalidation, aggregate-budget, bounded-lifetime, concurrent + refresh, incomplete reducer state, workspace propagation, and presentation + tests were added. The focused set passed 32 tests with no failures. +- `make check` passed. +- `make build-app` completed with no errors or warnings. +- A same-machine standard-suite comparison passed on both sides: + - exact #644 head `978b7b59`: `make test-app`, 2,160 tests, 0 failures; + - follow-up working tree: `make test-app`, 2,170 tests, 0 failures. + +Non-standard isolated test selections exposed three existing +`RepositoriesFeatureTests` timing/isolation failures on both the follow-up and a +clean clone of exact #644. Two tests consumed their expected delegate actions +but did not finish the remaining persistence/detection effects. They now call +`store.finish()`. The concurrency test polled startup with 100 `Task.yield()` +calls; it now waits for the first real fetch through an `AsyncGate`, uses a +`TestClock` to hold one fetch while the other starts, and then advances the +clock to complete both. The three tests passed independently on both trees and +passed all 15 runs in a five-iteration repetition. + +## Deviation from the initial implementation + +The first cache implementation used an actor. Running the cache tests in +parallel exposed a Swift runtime `EXC_BAD_ACCESS` while destroying the array of +cache updates after an actor call, even though each test passed in isolation. +The final cache uses `Synchronization.Mutex` for short in-memory lookups and +writes and performs all metadata and file I/O outside the lock. A 16-task +concurrent-refresh regression test covers the final boundary. + +The requested folder number 054 was already occupied by +`054-native-settings-navigation`, and 055 belongs to the open Agent Profile +runtime work. This topic therefore uses the next collision-free number, 056, +plus the requested date-qualified slug. diff --git a/docs-ai/README.md b/docs-ai/README.md index 32310e88..cd857c72 100644 --- a/docs-ai/README.md +++ b/docs-ai/README.md @@ -106,3 +106,4 @@ agent-facing manual for that). | 052 | [sidebar-context-menus](052-sidebar-context-menus/000-plan.md) | 2026-07-27 | Sidebar context menu overhaul: worktree terminal actions, header/workspace path actions, PR click-through | | 053 | [agent-profiles](053-agent-profiles/000-plan.md) | 2026-07-29 | Agent Profile V1: preset-first Claude Code/Codex launch profiles, opt-in account isolation, path routing, and future handoff seam | | 054 | [native-settings-navigation](054-native-settings-navigation/000-plan.md) | 2026-07-30 | SwiftUI-owned singleton Settings window, native sidebar/detail navigation, and Agent Profile drill-ins | +| 056 | [performance-optimization-2026-08](056-performance-optimization-2026-08/000-plan.md) | 2026-08-01 | Review and integration standard for the August performance series; exact cached and bounded untracked-line counts | diff --git a/docs/components/diff-view.md b/docs/components/diff-view.md index e99a10d4..8d6cf187 100644 --- a/docs/components/diff-view.md +++ b/docs/components/diff-view.md @@ -94,6 +94,13 @@ Repositories can show **line-change badges** (additions/deletions) on worktree rows, controlled per repo by `observeLineDiffsAutomatically` (on by default). Disable it for very large repos if it's expensive. +Prowl caches line counts for unchanged untracked files. On a cold refresh it +scans at most 32 MiB of uncached untracked content across the worktree. If more +content remains, the additions label ends in an ellipsis (`+N…`, or `+…` when +no additions were counted yet), and its tooltip identifies how many untracked +files were omitted. The badge stays available to open Show Diff, which still +lists every changed file. Tracked additions and deletions remain exact. + ## Availability Diff is a **git-only** feature — it's unavailable for plain (non-git) folders. diff --git a/supacode/Clients/Git/GitClient.swift b/supacode/Clients/Git/GitClient.swift index 8af3b03c..2a1a17aa 100644 --- a/supacode/Clients/Git/GitClient.swift +++ b/supacode/Clients/Git/GitClient.swift @@ -422,7 +422,7 @@ struct GitClient { return "HEAD" } - nonisolated func lineChanges(at worktreeURL: URL) async -> (added: Int, removed: Int)? { + nonisolated func lineChanges(at worktreeURL: URL) async -> GitLineChanges? { if await isWorktreeIndexLocked(worktreeURL) { return nil } @@ -438,8 +438,12 @@ struct GitClient { ) let tracked = parseShortstat(try await diffOutput) let untrackedPaths = parseNULFileList(try await untrackedOutput) - let untrackedLines = Self.countLinesInFiles(untrackedPaths, relativeTo: worktreeURL) - return (added: tracked.added + untrackedLines, removed: tracked.removed) + let untracked = Self.countLinesInFiles(untrackedPaths, relativeTo: worktreeURL) + return GitLineChanges( + added: tracked.added + untracked.lines, + removed: tracked.removed, + skippedUntrackedFileCount: untracked.skippedFileCount + ) } catch { return nil } @@ -458,47 +462,131 @@ struct GitClient { return Int(Self.bigEndianUInt32(from: header, offset: 8)) } - nonisolated static func countLinesInFiles(_ relativePaths: [String], relativeTo base: URL) -> Int { - var total = 0 - for relativePath in relativePaths { + nonisolated static let untrackedLineCountByteBudget = 32 * 1_024 * 1_024 + + nonisolated static func countLinesInFiles( + _ relativePaths: [String], + relativeTo base: URL, + cache: UntrackedLineCountCache = .shared, + byteBudget: Int = untrackedLineCountByteBudget + ) -> UntrackedLineCountResult { + struct FileToCount { + let relativePath: String + let url: URL + let fingerprint: UntrackedLineFileFingerprint + } + + let resourceKeys: Set = [ + .contentModificationDateKey, + .fileResourceIdentifierKey, + .fileSizeKey, + ] + var skippedFileCount = 0 + let files = relativePaths.compactMap { relativePath -> FileToCount? in let fileURL = base.appending(path: relativePath) - total += Self.countLines(in: fileURL) ?? 0 + guard let values = try? fileURL.resourceValues(forKeys: resourceKeys), + let byteCount = values.fileSize, + let modificationDate = values.contentModificationDate + else { + if FileManager.default.fileExists(atPath: fileURL.path(percentEncoded: false)) { + skippedFileCount += 1 + } + return nil + } + return FileToCount( + relativePath: relativePath, + url: fileURL, + fingerprint: UntrackedLineFileFingerprint( + byteCount: byteCount, + modificationDate: modificationDate, + resourceIdentifier: values.fileResourceIdentifier.map { String(describing: $0) } + ) + ) + } + let worktreeKey = base.standardizedFileURL.path(percentEncoded: false) + let cacheFiles = files.map { + UntrackedLineCacheFile(relativePath: $0.relativePath, fingerprint: $0.fingerprint) + } + let cachedValues = cache.cachedValues(for: cacheFiles, worktreeKey: worktreeKey) + + var total = 0 + var misses: [FileToCount] = [] + for file in files { + switch cachedValues[file.relativePath] { + case .text(let lines): + total += lines + case .binary: + break + case nil: + misses.append(file) + } + } + + var remainingByteBudget = max(0, byteBudget) + var cacheUpdates: [UntrackedLineCacheUpdate] = [] + for file in misses.sorted(by: { lhs, rhs in + if lhs.fingerprint.byteCount == rhs.fingerprint.byteCount { + return lhs.relativePath < rhs.relativePath + } + return lhs.fingerprint.byteCount < rhs.fingerprint.byteCount + }) { + guard file.fingerprint.byteCount <= remainingByteBudget else { + skippedFileCount += 1 + continue + } + switch Self.countLines(in: file.url, maximumByteCount: remainingByteBudget) { + case .text(let lines, let bytesRead): + total += lines + remainingByteBudget -= bytesRead + cacheUpdates.append( + UntrackedLineCacheUpdate( + relativePath: file.relativePath, + fingerprint: file.fingerprint, + value: .text(lines) + )) + case .binary(let bytesRead): + remainingByteBudget -= bytesRead + cacheUpdates.append( + UntrackedLineCacheUpdate( + relativePath: file.relativePath, + fingerprint: file.fingerprint, + value: .binary + )) + case .budgetExceeded: + remainingByteBudget = 0 + skippedFileCount += 1 + case .unavailable(let bytesRead): + remainingByteBudget -= bytesRead + skippedFileCount += 1 + } } - return total + cache.store(cacheUpdates, worktreeKey: worktreeKey) + return UntrackedLineCountResult(lines: total, skippedFileCount: skippedFileCount) } nonisolated private static func bigEndianUInt32(from data: Data, offset: Int) -> UInt32 { data[offset..<(offset + 4)].reduce(UInt32(0)) { ($0 << 8) | UInt32($1) } } - /// Untracked files at or above this size contribute nothing to the diff badge. - /// - /// The badge counts the lines you are adding, and a file this large is not - /// something anyone typed. Profiling captures are the case that prompted it: - /// a single `sample(1)` output runs to tens of megabytes, and every byte of it - /// was being read on each refresh to produce a number no one wants. Source - /// files sit orders of magnitude below the threshold, so nothing a reader would - /// call a change is affected. - /// - /// Chosen over a `.gitignore` pattern deliberately. A pattern has to name the - /// file, so it fixes one repository for one spelling and misses the next - /// capture that lands under a different name. - nonisolated static let untrackedLineCountByteLimit = 2 * 1_024 * 1_024 - - nonisolated private static func countLines(in fileURL: URL) -> Int? { - // Checked before opening the file, so an oversized one costs a stat rather - // than a full read. `nil` already means "not worth counting" here; binary - // files reach the same answer through the NUL probe below. - if let fileSize = try? fileURL.resourceValues(forKeys: [.fileSizeKey]).fileSize, - fileSize >= untrackedLineCountByteLimit - { - return nil + nonisolated private enum FileLineCountResult { + case text(lines: Int, bytesRead: Int) + case binary(bytesRead: Int) + case budgetExceeded + case unavailable(bytesRead: Int) + } + + nonisolated private static func countLines( + in fileURL: URL, + maximumByteCount: Int + ) -> FileLineCountResult { + guard let handle = try? FileHandle(forReadingFrom: fileURL) else { + return .unavailable(bytesRead: 0) } - guard let handle = try? FileHandle(forReadingFrom: fileURL) else { return nil } defer { try? handle.close() } let binaryProbeByteCount = 8_192 let chunkByteCount = 64 * 1_024 + var bytesRead = 0 var probedByteCount = 0 var lineCount = 0 var isEmpty = true @@ -507,18 +595,24 @@ struct GitClient { while true { let chunk: Data do { - guard let readChunk = try handle.read(upToCount: chunkByteCount) else { break } + // The extra byte distinguishes a file exactly at the budget from one + // that grew after its metadata fingerprint was collected. + let allowedReadCount = min(chunkByteCount, maximumByteCount - bytesRead + 1) + guard allowedReadCount > 0 else { return .budgetExceeded } + guard let readChunk = try handle.read(upToCount: allowedReadCount) else { break } chunk = readChunk } catch { - return nil + return .unavailable(bytesRead: bytesRead) } guard !chunk.isEmpty else { break } + bytesRead += chunk.count + guard bytesRead <= maximumByteCount else { return .budgetExceeded } isEmpty = false if probedByteCount < binaryProbeByteCount { let remainingProbeCount = binaryProbeByteCount - probedByteCount let probe = chunk.prefix(remainingProbeCount) - if probe.containsByte(0x00) { return nil } + if probe.containsByte(0x00) { return .binary(bytesRead: bytesRead) } probedByteCount += probe.count } lineCount += chunk.countOccurrences(of: 0x0A) @@ -528,7 +622,7 @@ struct GitClient { if !isEmpty, lastByte != 0x0A { lineCount += 1 } - return lineCount + return .text(lines: lineCount, bytesRead: bytesRead) } nonisolated private static func resolveGitDirectory(for worktreeURL: URL) -> URL? { @@ -1437,8 +1531,8 @@ struct GitClient { /// for 300 s attributed roughly one whole core to exactly that path inside /// `countLines` — 31% in `Sequence.reduce`, 30% in `Data.Iterator.next`, 22% in /// value witnesses — about 72% of everything the process was burning, while -/// counting lines in untracked files. `memchr` covers the same bytes in one -/// vectorized pass over contiguous memory. +/// counting lines in untracked files. `memchr` scans sparse matches in vectorized +/// segments; a raw-pointer fallback bounds the cost when matches are dense. extension Data { /// Occurrences of `byte` in the whole buffer. fileprivate nonisolated func countOccurrences(of byte: UInt8) -> Int { @@ -1446,12 +1540,21 @@ extension Data { guard let base = raw.baseAddress, !raw.isEmpty else { return 0 } var count = 0 var scanned = 0 + let denseMatchLimit = 2_048 while scanned < raw.count, let hit = memchr(base + scanned, Int32(byte), raw.count - scanned) { // memchr returns the hit itself, so resume one byte past it. scanned = base.distance(to: UnsafeRawPointer(hit)) + 1 count += 1 + if count == denseMatchLimit { + let remaining = raw.count - scanned + let bytes = (base + scanned).assumingMemoryBound(to: UInt8.self) + for index in 0.. + + init(maximumWorktreeCount: Int = 128) { + precondition(maximumWorktreeCount > 0) + state = Mutex(State(maximumWorktreeCount: maximumWorktreeCount)) + } + + func cachedValues( + for files: [UntrackedLineCacheFile], + worktreeKey: String + ) -> [String: CachedUntrackedLineCount] { + state.withLock { state in + guard !files.isEmpty else { + state.entriesByWorktree.removeValue(forKey: worktreeKey) + state.lastAccessByWorktree.removeValue(forKey: worktreeKey) + return [:] + } + let currentPaths = Set(files.map(\.relativePath)) + var entries = state.entriesByWorktree[worktreeKey, default: [:]] + entries = entries.filter { currentPaths.contains($0.key) } + state.entriesByWorktree[worktreeKey] = entries + touch(worktreeKey, state: &state) + trimIfNeeded(state: &state) + + var result: [String: CachedUntrackedLineCount] = [:] + for file in files { + guard let entry = entries[file.relativePath], entry.fingerprint == file.fingerprint else { + continue + } + result[file.relativePath] = entry.value + } + return result + } + } + + func store( + _ updates: [UntrackedLineCacheUpdate], + worktreeKey: String + ) { + guard !updates.isEmpty else { return } + state.withLock { state in + var entries = state.entriesByWorktree[worktreeKey, default: [:]] + for update in updates { + entries[update.relativePath] = Entry( + fingerprint: update.fingerprint, + value: update.value + ) + } + state.entriesByWorktree[worktreeKey] = entries + touch(worktreeKey, state: &state) + trimIfNeeded(state: &state) + } + } + + private func touch(_ worktreeKey: String, state: inout State) { + state.accessSequence &+= 1 + state.lastAccessByWorktree[worktreeKey] = state.accessSequence + } + + private func trimIfNeeded(state: inout State) { + while state.entriesByWorktree.count > state.maximumWorktreeCount, + let leastRecentlyUsed = state.lastAccessByWorktree.min(by: { $0.value < $1.value })?.key + { + state.entriesByWorktree.removeValue(forKey: leastRecentlyUsed) + state.lastAccessByWorktree.removeValue(forKey: leastRecentlyUsed) + } + } +} diff --git a/supacode/Clients/Repositories/GitClientDependency.swift b/supacode/Clients/Repositories/GitClientDependency.swift index 977bdfe2..ca553870 100644 --- a/supacode/Clients/Repositories/GitClientDependency.swift +++ b/supacode/Clients/Repositories/GitClientDependency.swift @@ -31,7 +31,7 @@ struct GitClientDependency: Sendable { -> LocalBranchDeletionOutcome var isBareRepository: @Sendable (_ repoRoot: URL) async throws -> Bool var branchName: @Sendable (URL) async -> String? - var lineChanges: @Sendable (URL) async -> (added: Int, removed: Int)? + var lineChanges: @Sendable (URL) async -> GitLineChanges? var renameBranch: @Sendable (_ worktreeURL: URL, _ branchName: String) async throws -> Void var repositoryWebURL: @Sendable (_ repositoryRoot: URL) async -> URL? var githubRemoteInfos: @Sendable (_ repositoryRoot: URL) async -> [GithubRemoteInfo] diff --git a/supacode/Domain/LineChangeBadgePresentation.swift b/supacode/Domain/LineChangeBadgePresentation.swift new file mode 100644 index 00000000..88f39139 --- /dev/null +++ b/supacode/Domain/LineChangeBadgePresentation.swift @@ -0,0 +1,29 @@ +import Foundation + +nonisolated struct LineChangeBadgePresentation: Equatable, Sendable { + let addedText: String + let removedText: String + let incompleteCountDescription: String? + let accessibilityLabel: String + + init( + addedLines: Int, + removedLines: Int, + skippedUntrackedFileCount: Int + ) { + removedText = "-\(removedLines)" + guard skippedUntrackedFileCount > 0 else { + addedText = "+\(addedLines)" + incompleteCountDescription = nil + accessibilityLabel = "\(addedLines) added lines, \(removedLines) removed lines" + return + } + + addedText = addedLines == 0 ? "+…" : "+\(addedLines)…" + let fileNoun = skippedUntrackedFileCount == 1 ? "file was" : "files were" + let omission = "\(skippedUntrackedFileCount) untracked \(fileNoun) not counted." + incompleteCountDescription = "Addition count is incomplete because \(omission)" + accessibilityLabel = + "Addition count incomplete, \(addedLines) lines counted; \(omission) \(removedLines) removed lines." + } +} diff --git a/supacode/Domain/WorktreeInfoEntry.swift b/supacode/Domain/WorktreeInfoEntry.swift index 68914332..c536c955 100644 --- a/supacode/Domain/WorktreeInfoEntry.swift +++ b/supacode/Domain/WorktreeInfoEntry.swift @@ -4,8 +4,21 @@ struct WorktreeInfoEntry: Equatable, Hashable { var addedLines: Int? var removedLines: Int? var pullRequest: GithubPullRequest? + var skippedUntrackedFileCount: Int + + init( + addedLines: Int? = nil, + removedLines: Int? = nil, + pullRequest: GithubPullRequest? = nil, + skippedUntrackedFileCount: Int = 0 + ) { + self.addedLines = addedLines + self.removedLines = removedLines + self.pullRequest = pullRequest + self.skippedUntrackedFileCount = skippedUntrackedFileCount + } var isEmpty: Bool { - addedLines == nil && removedLines == nil && pullRequest == nil + addedLines == nil && removedLines == nil && pullRequest == nil && skippedUntrackedFileCount == 0 } } diff --git a/supacode/Features/Repositories/Reducer/RepositoriesFeature+CoreReducer.swift b/supacode/Features/Repositories/Reducer/RepositoriesFeature+CoreReducer.swift index 306a5ae3..927809a4 100644 --- a/supacode/Features/Repositories/Reducer/RepositoriesFeature+CoreReducer.swift +++ b/supacode/Features/Repositories/Reducer/RepositoriesFeature+CoreReducer.swift @@ -937,16 +937,14 @@ extension RepositoriesFeature { let previousLineChanges = normalizedLineChanges(state.worktreeInfoByID[worktreeID]) return .run { send in if let changes = await gitClient.lineChanges(worktreeURL) { - let nextLineChanges = normalizedLineChanges( - added: changes.added, removed: changes.removed) - guard !lineChangesEqual(nextLineChanges, previousLineChanges) else { + let nextLineChanges = normalizedLineChanges(changes) + guard nextLineChanges != previousLineChanges else { return } await send( .worktreeLineChangesLoaded( worktreeID: worktreeID, - added: changes.added, - removed: changes.removed + changes: changes ) ) } @@ -996,11 +994,10 @@ extension RepositoriesFeature { updateWorktreeName(worktreeID, name: name, state: &state) return .none - case .worktreeLineChangesLoaded(let worktreeID, let added, let removed): + case .worktreeLineChangesLoaded(let worktreeID, let changes): updateWorktreeLineChanges( worktreeID: worktreeID, - added: added, - removed: removed, + changes: changes, state: &state ) return .none diff --git a/supacode/Features/Repositories/Reducer/RepositoriesFeature+WorkspaceChildren.swift b/supacode/Features/Repositories/Reducer/RepositoriesFeature+WorkspaceChildren.swift index c9202e2b..e6556eea 100644 --- a/supacode/Features/Repositories/Reducer/RepositoriesFeature+WorkspaceChildren.swift +++ b/supacode/Features/Repositories/Reducer/RepositoriesFeature+WorkspaceChildren.swift @@ -49,8 +49,7 @@ extension RepositoriesFeature { return WorkspaceChildInfoUpdate( id: child.id, branch: branch, - added: changes?.added, - removed: changes?.removed, + lineChanges: changes, pullRequest: pullRequest ) } @@ -106,13 +105,14 @@ func applyWorkspaceChildrenInfo( } var entry = state.workspaceChildInfoByID[update.id] ?? WorktreeInfoEntry() - if let added = update.added, let removed = update.removed, !(added == 0 && removed == 0) { - entry.addedLines = added - entry.removedLines = removed + if let changes = update.lineChanges, !changes.isEmpty { + entry.addedLines = changes.added + entry.removedLines = changes.removed } else { entry.addedLines = nil entry.removedLines = nil } + entry.skippedUntrackedFileCount = update.lineChanges?.skippedUntrackedFileCount ?? 0 entry.pullRequest = update.pullRequest if entry.isEmpty { state.workspaceChildInfoByID.removeValue(forKey: update.id) diff --git a/supacode/Features/Repositories/Reducer/RepositoriesFeature+WorktreeState.swift b/supacode/Features/Repositories/Reducer/RepositoriesFeature+WorktreeState.swift index 32f2cc89..e6858987 100644 --- a/supacode/Features/Repositories/Reducer/RepositoriesFeature+WorktreeState.swift +++ b/supacode/Features/Repositories/Reducer/RepositoriesFeature+WorktreeState.swift @@ -251,18 +251,18 @@ func updateWorktreeName( @discardableResult func updateWorktreeLineChanges( worktreeID: Worktree.ID, - added: Int, - removed: Int, + changes: GitLineChanges, state: inout RepositoriesFeature.State ) -> Bool { var entry = state.worktreeInfoByID[worktreeID] ?? WorktreeInfoEntry() - if added == 0 && removed == 0 { + if changes.isEmpty { entry.addedLines = nil entry.removedLines = nil } else { - entry.addedLines = added - entry.removedLines = removed + entry.addedLines = changes.added + entry.removedLines = changes.removed } + entry.skippedUntrackedFileCount = changes.skippedUntrackedFileCount let previousEntry = state.worktreeInfoByID[worktreeID] if entry.isEmpty { guard previousEntry != nil else { @@ -292,33 +292,18 @@ func updateWorktreePullRequest( } } -nonisolated func normalizedLineChanges(_ entry: WorktreeInfoEntry?) -> (added: Int, removed: Int)? { +nonisolated func normalizedLineChanges(_ entry: WorktreeInfoEntry?) -> GitLineChanges? { guard let added = entry?.addedLines, let removed = entry?.removedLines else { return nil } - return normalizedLineChanges(added: added, removed: removed) + return normalizedLineChanges( + GitLineChanges( + added: added, + removed: removed, + skippedUntrackedFileCount: entry?.skippedUntrackedFileCount ?? 0 + )) } -nonisolated func normalizedLineChanges( - added: Int, - removed: Int -) -> (added: Int, removed: Int)? { - guard added != 0 || removed != 0 else { - return nil - } - return (added, removed) -} - -nonisolated func lineChangesEqual( - _ lhs: (added: Int, removed: Int)?, - _ rhs: (added: Int, removed: Int)? -) -> Bool { - switch (lhs, rhs) { - case (nil, nil): - return true - case (.some(let lhs), .some(let rhs)): - return lhs.added == rhs.added && lhs.removed == rhs.removed - default: - return false - } +nonisolated func normalizedLineChanges(_ changes: GitLineChanges) -> GitLineChanges? { + changes.isEmpty ? nil : changes } diff --git a/supacode/Features/Repositories/Reducer/RepositoriesFeature.swift b/supacode/Features/Repositories/Reducer/RepositoriesFeature.swift index f252bd6f..8e928076 100644 --- a/supacode/Features/Repositories/Reducer/RepositoriesFeature.swift +++ b/supacode/Features/Repositories/Reducer/RepositoriesFeature.swift @@ -72,11 +72,10 @@ struct ForceDeleteBranchRequest: Equatable { // Result of refreshing one workspace child repository's live status: current // branch, uncommitted diff counts, and (when GitHub integration is available) // the PR for that branch. -struct WorkspaceChildInfoUpdate: Equatable, Sendable { +nonisolated struct WorkspaceChildInfoUpdate: Equatable, Sendable { let id: String let branch: String? - let added: Int? - let removed: Int? + let lineChanges: GitLineChanges? let pullRequest: GithubPullRequest? } @@ -470,7 +469,7 @@ struct RepositoriesFeature { case presentAlert(title: String, message: String) case worktreeInfoEvent(WorktreeInfoWatcherClient.Event) case worktreeBranchNameLoaded(worktreeID: Worktree.ID, name: String) - case worktreeLineChangesLoaded(worktreeID: Worktree.ID, added: Int, removed: Int) + case worktreeLineChangesLoaded(worktreeID: Worktree.ID, changes: GitLineChanges) case workspaceChildrenInfoLoaded([WorkspaceChildInfoUpdate]) case showToast(StatusToast) case dismissToast diff --git a/supacode/Features/Repositories/Views/WorktreeRow.swift b/supacode/Features/Repositories/Views/WorktreeRow.swift index b58925d0..95dbcec4 100644 --- a/supacode/Features/Repositories/Views/WorktreeRow.swift +++ b/supacode/Features/Repositories/Views/WorktreeRow.swift @@ -88,10 +88,19 @@ struct WorktreeRow: View { ) let displayAddedLines = info?.addedLines let displayRemovedLines = info?.removedLines + let lineChangePresentation: LineChangeBadgePresentation? = + if let displayAddedLines, let displayRemovedLines { + LineChangeBadgePresentation( + addedLines: displayAddedLines, + removedLines: displayRemovedLines, + skippedUntrackedFileCount: info?.skippedUntrackedFileCount ?? 0 + ) + } else { + nil + } let mergeReadiness = pullRequestMergeReadiness(for: display.pullRequest) let isQueued = display.pullRequest.flatMap(PullRequestMergeQueueStatus.init(pullRequest:)) != nil - let hasChangeCounts = displayAddedLines != nil && displayRemovedLines != nil let showsPullRequestTag = display.pullRequest != nil && display.pullRequestBadgeStyle != nil let nameColor = colorScheme == .dark ? Color.white : Color.primary let detailText = worktreeName.isEmpty ? name : worktreeName @@ -163,23 +172,28 @@ struct WorktreeRow: View { if isRunScriptRunning { RunScriptIndicator(onStop: onStopRunScript) } - if hasChangeCounts, let displayAddedLines, let displayRemovedLines { + if let lineChangePresentation { Button { onDiffTap?() } label: { WorktreeRowChangeCountView( - addedLines: displayAddedLines, - removedLines: displayRemovedLines, + presentation: lineChangePresentation, isSelected: isSelected, ) } .buttonStyle(.plain) .help( - AppShortcuts.helpText( - title: "Show Diff", - commandID: AppShortcuts.CommandID.showDiff, - in: resolvedKeybindings - )) + [ + AppShortcuts.helpText( + title: "Show Diff", + commandID: AppShortcuts.CommandID.showDiff, + in: resolvedKeybindings + ), + lineChangePresentation.incompleteCountDescription, + ] + .compactMap { $0 } + .joined(separator: "\n") + ) } } WorktreeRowInfoView( @@ -421,15 +435,14 @@ private struct WorktreeRowPreview: View { // MARK: - Subviews private struct WorktreeRowChangeCountView: View { - let addedLines: Int - let removedLines: Int + let presentation: LineChangeBadgePresentation let isSelected: Bool var body: some View { HStack(spacing: 4) { - Text("+\(addedLines)") + Text(presentation.addedText) .foregroundStyle(.green) - Text("-\(removedLines)") + Text(presentation.removedText) .foregroundStyle(.red) .baselineOffset(-1) } @@ -438,6 +451,8 @@ private struct WorktreeRowChangeCountView: View { .padding(.horizontal, 4) .padding(.vertical, 0) .fixedSize(horizontal: true, vertical: false) + .accessibilityElement(children: .ignore) + .accessibilityLabel(presentation.accessibilityLabel) .overlay { Capsule() .stroke( diff --git a/supacode/Support/CustomDump+Extensions.swift b/supacode/Support/CustomDump+Extensions.swift index a9f7feb8..fa3ad875 100644 --- a/supacode/Support/CustomDump+Extensions.swift +++ b/supacode/Support/CustomDump+Extensions.swift @@ -83,6 +83,7 @@ extension WorktreeInfoEntry: CustomDumpRepresentable { ( added: addedLines, removed: removedLines, + skippedUntrackedFiles: skippedUntrackedFileCount, hasPR: pullRequest != nil ) } diff --git a/supacodeTests/GitClientLineChangesTests.swift b/supacodeTests/GitClientLineChangesTests.swift index ec5dfaf0..857d71dc 100644 --- a/supacodeTests/GitClientLineChangesTests.swift +++ b/supacodeTests/GitClientLineChangesTests.swift @@ -270,7 +270,7 @@ struct GitClientLineChangesTests { try "a\nb\nc\n".write(to: tempRoot.appending(path: "a.txt"), atomically: true, encoding: .utf8) try "x\ny\n".write(to: tempRoot.appending(path: "b.txt"), atomically: true, encoding: .utf8) - let count = GitClient.countLinesInFiles(["a.txt", "b.txt"], relativeTo: tempRoot) + let count = GitClient.countLinesInFiles(["a.txt", "b.txt"], relativeTo: tempRoot).lines #expect(count == 5) } @@ -282,7 +282,7 @@ struct GitClientLineChangesTests { try "hello".write(to: tempRoot.appending(path: "single.txt"), atomically: true, encoding: .utf8) try "a\nb".write(to: tempRoot.appending(path: "multi.txt"), atomically: true, encoding: .utf8) - let count = GitClient.countLinesInFiles(["single.txt", "multi.txt"], relativeTo: tempRoot) + let count = GitClient.countLinesInFiles(["single.txt", "multi.txt"], relativeTo: tempRoot).lines #expect(count == 3) } @@ -297,38 +297,44 @@ struct GitClientLineChangesTests { binary.append(contentsOf: Data(repeating: 0x0A, count: 50)) try binary.write(to: tempRoot.appending(path: "img.bin")) - let count = GitClient.countLinesInFiles(["ok.txt", "img.bin"], relativeTo: tempRoot) + let count = GitClient.countLinesInFiles(["ok.txt", "img.bin"], relativeTo: tempRoot).lines #expect(count == 2) } - /// A capture-sized untracked file must not be read at all. The badge counts - /// lines you are adding, and nobody typed 2 MiB of `sample(1)` output. - @Test func countLinesInFilesSkipsFilesAtOrAboveTheByteLimit() throws { + /// The former per-file cutoff silently hid readable text changes. A file at + /// that boundary must remain exact when the refresh-wide budget allows it. + @Test func countLinesInFilesCountsFilesAtTheLegacyByteLimit() throws { let fileManager = FileManager.default let tempRoot = fileManager.temporaryDirectory.appending(path: UUID().uuidString) defer { try? fileManager.removeItem(at: tempRoot) } try fileManager.createDirectory(at: tempRoot, withIntermediateDirectories: true) try "a\nb\n".write(to: tempRoot.appending(path: "small.txt"), atomically: true, encoding: .utf8) - // Text, not binary: the NUL probe would pass it and count every newline. - let oversized = Data(repeating: 0x0A, count: GitClient.untrackedLineCountByteLimit) - try oversized.write(to: tempRoot.appending(path: "sample.txt")) - - let count = GitClient.countLinesInFiles(["small.txt", "sample.txt"], relativeTo: tempRoot) - #expect(count == 2, "Only the small file should contribute") + let legacyLimit = 2 * 1_024 * 1_024 + // Text, not binary: every newline must contribute to the visible badge. + let largeText = Data(repeating: 0x0A, count: legacyLimit) + try largeText.write(to: tempRoot.appending(path: "sample.txt")) + + let count = GitClient.countLinesInFiles( + ["small.txt", "sample.txt"], + relativeTo: tempRoot, + byteBudget: legacyLimit + 16 + ).lines + #expect(count == legacyLimit + 2) } - /// The boundary is the whole point of the guard, so pin the side that must - /// still count. One byte under the limit is an ordinary file. - @Test func countLinesInFilesStillCountsJustUnderTheByteLimit() throws { + /// Keep coverage on both sides of the removed cutoff so a future optimization + /// cannot accidentally reintroduce the old discontinuity. + @Test func countLinesInFilesCountsJustUnderTheLegacyByteLimit() throws { let fileManager = FileManager.default let tempRoot = fileManager.temporaryDirectory.appending(path: UUID().uuidString) defer { try? fileManager.removeItem(at: tempRoot) } try fileManager.createDirectory(at: tempRoot, withIntermediateDirectories: true) - let justUnder = Data(repeating: 0x0A, count: GitClient.untrackedLineCountByteLimit - 1) + let legacyLimit = 2 * 1_024 * 1_024 + let justUnder = Data(repeating: 0x0A, count: legacyLimit - 1) try justUnder.write(to: tempRoot.appending(path: "big.txt")) - let count = GitClient.countLinesInFiles(["big.txt"], relativeTo: tempRoot) - #expect(count == GitClient.untrackedLineCountByteLimit - 1) + let count = GitClient.countLinesInFiles(["big.txt"], relativeTo: tempRoot).lines + #expect(count == legacyLimit - 1) } @Test func countLinesInFilesSkipsMissingFiles() throws { @@ -338,7 +344,7 @@ struct GitClientLineChangesTests { try fileManager.createDirectory(at: tempRoot, withIntermediateDirectories: true) try "a\nb\n".write(to: tempRoot.appending(path: "exists.txt"), atomically: true, encoding: .utf8) - let count = GitClient.countLinesInFiles(["exists.txt", "gone.txt"], relativeTo: tempRoot) + let count = GitClient.countLinesInFiles(["exists.txt", "gone.txt"], relativeTo: tempRoot).lines #expect(count == 2) } @@ -354,7 +360,7 @@ struct GitClientLineChangesTests { let content = String(repeating: "line\n", count: 40_000) try content.write(to: tempRoot.appending(path: "big.txt"), atomically: true, encoding: .utf8) - #expect(GitClient.countLinesInFiles(["big.txt"], relativeTo: tempRoot) == 40_000) + #expect(GitClient.countLinesInFiles(["big.txt"], relativeTo: tempRoot).lines == 40_000) } /// A newline landing on the final byte of a chunk is the boundary case: the scan must @@ -370,7 +376,7 @@ struct GitClientLineChangesTests { bytes.append(contentsOf: Data("second\n".utf8)) try bytes.write(to: tempRoot.appending(path: "boundary.txt")) - #expect(GitClient.countLinesInFiles(["boundary.txt"], relativeTo: tempRoot) == 2) + #expect(GitClient.countLinesInFiles(["boundary.txt"], relativeTo: tempRoot).lines == 2) } /// Binary detection probes only the first 8 KiB. A NUL past that window has always @@ -385,7 +391,7 @@ struct GitClientLineChangesTests { bytes.append(0x0A) try bytes.write(to: tempRoot.appending(path: "late-nul.txt")) - #expect(GitClient.countLinesInFiles(["late-nul.txt"], relativeTo: tempRoot) == 1) + #expect(GitClient.countLinesInFiles(["late-nul.txt"], relativeTo: tempRoot).lines == 1) } @Test func countLinesInFilesCountsAnEmptyFileAsZero() throws { @@ -395,7 +401,125 @@ struct GitClientLineChangesTests { try fileManager.createDirectory(at: tempRoot, withIntermediateDirectories: true) try Data().write(to: tempRoot.appending(path: "empty.txt")) - #expect(GitClient.countLinesInFiles(["empty.txt"], relativeTo: tempRoot) == 0) + #expect(GitClient.countLinesInFiles(["empty.txt"], relativeTo: tempRoot).lines == 0) + } + + @Test func countLinesInFilesReusesCachedCountsWithoutSpendingTheRefreshBudget() throws { + let fileManager = FileManager.default + let tempRoot = fileManager.temporaryDirectory.appending(path: UUID().uuidString) + defer { try? fileManager.removeItem(at: tempRoot) } + try fileManager.createDirectory(at: tempRoot, withIntermediateDirectories: true) + try "a\nb\n".write(to: tempRoot.appending(path: "cached.txt"), atomically: true, encoding: .utf8) + let cache = UntrackedLineCountCache() + + let cold = GitClient.countLinesInFiles( + ["cached.txt"], + relativeTo: tempRoot, + cache: cache, + byteBudget: 4 + ) + let warm = GitClient.countLinesInFiles( + ["cached.txt"], + relativeTo: tempRoot, + cache: cache, + byteBudget: 0 + ) + + #expect(cold == UntrackedLineCountResult(lines: 2, skippedFileCount: 0)) + #expect(warm == cold) + } + + @Test func countLinesInFilesInvalidatesCachedCountsWhenTheFileChanges() throws { + let fileManager = FileManager.default + let tempRoot = fileManager.temporaryDirectory.appending(path: UUID().uuidString) + defer { try? fileManager.removeItem(at: tempRoot) } + try fileManager.createDirectory(at: tempRoot, withIntermediateDirectories: true) + let fileURL = tempRoot.appending(path: "changing.txt") + try "a\nb\n".write(to: fileURL, atomically: true, encoding: .utf8) + let cache = UntrackedLineCountCache() + + let initial = GitClient.countLinesInFiles( + ["changing.txt"], + relativeTo: tempRoot, + cache: cache, + byteBudget: 4 + ) + try "a\nb\nc\n".write(to: fileURL, atomically: true, encoding: .utf8) + let invalidated = GitClient.countLinesInFiles( + ["changing.txt"], + relativeTo: tempRoot, + cache: cache, + byteBudget: 0 + ) + + #expect(initial == UntrackedLineCountResult(lines: 2, skippedFileCount: 0)) + #expect(invalidated == UntrackedLineCountResult(lines: 0, skippedFileCount: 1)) + } + + @Test func countLinesInFilesAppliesOneBudgetAcrossAllCacheMisses() throws { + let fileManager = FileManager.default + let tempRoot = fileManager.temporaryDirectory.appending(path: UUID().uuidString) + defer { try? fileManager.removeItem(at: tempRoot) } + try fileManager.createDirectory(at: tempRoot, withIntermediateDirectories: true) + try "a\n".write(to: tempRoot.appending(path: "small.txt"), atomically: true, encoding: .utf8) + try "b\nc\n".write(to: tempRoot.appending(path: "large.txt"), atomically: true, encoding: .utf8) + + let result = GitClient.countLinesInFiles( + ["large.txt", "small.txt"], + relativeTo: tempRoot, + cache: UntrackedLineCountCache(), + byteBudget: 2 + ) + + #expect(result == UntrackedLineCountResult(lines: 1, skippedFileCount: 1)) + } + + @Test func countLinesInFilesBoundsCachedWorktreeLifetime() throws { + let fileManager = FileManager.default + let tempRoot = fileManager.temporaryDirectory.appending(path: UUID().uuidString) + defer { try? fileManager.removeItem(at: tempRoot) } + let firstRoot = tempRoot.appending(path: "first", directoryHint: .isDirectory) + let secondRoot = tempRoot.appending(path: "second", directoryHint: .isDirectory) + try fileManager.createDirectory(at: firstRoot, withIntermediateDirectories: true) + try fileManager.createDirectory(at: secondRoot, withIntermediateDirectories: true) + try "a\n".write(to: firstRoot.appending(path: "file.txt"), atomically: true, encoding: .utf8) + try "b\n".write(to: secondRoot.appending(path: "file.txt"), atomically: true, encoding: .utf8) + let cache = UntrackedLineCountCache(maximumWorktreeCount: 1) + + _ = GitClient.countLinesInFiles( + ["file.txt"], relativeTo: firstRoot, cache: cache, byteBudget: 2) + _ = GitClient.countLinesInFiles( + ["file.txt"], relativeTo: secondRoot, cache: cache, byteBudget: 2) + let evicted = GitClient.countLinesInFiles( + ["file.txt"], relativeTo: firstRoot, cache: cache, byteBudget: 0) + + #expect(evicted == UntrackedLineCountResult(lines: 0, skippedFileCount: 1)) + } + + @Test func countLinesInFilesSupportsConcurrentRefreshes() async throws { + let fileManager = FileManager.default + let tempRoot = fileManager.temporaryDirectory.appending(path: UUID().uuidString) + defer { try? fileManager.removeItem(at: tempRoot) } + try fileManager.createDirectory(at: tempRoot, withIntermediateDirectories: true) + try "a\nb\n".write(to: tempRoot.appending(path: "shared.txt"), atomically: true, encoding: .utf8) + let cache = UntrackedLineCountCache() + + let results = await withTaskGroup(of: UntrackedLineCountResult.self) { group in + for _ in 0..<16 { + group.addTask { + GitClient.countLinesInFiles( + ["shared.txt"], relativeTo: tempRoot, cache: cache, byteBudget: 4) + } + } + var results: [UntrackedLineCountResult] = [] + for await result in group { + results.append(result) + } + return results + } + + #expect(results.count == 16) + #expect(results.allSatisfy { $0 == UntrackedLineCountResult(lines: 2, skippedFileCount: 0) }) } private func writeGitIndexHeader( diff --git a/supacodeTests/LineChangeBadgePresentationTests.swift b/supacodeTests/LineChangeBadgePresentationTests.swift new file mode 100644 index 00000000..405f3903 --- /dev/null +++ b/supacodeTests/LineChangeBadgePresentationTests.swift @@ -0,0 +1,51 @@ +import Testing + +@testable import supacode + +struct LineChangeBadgePresentationTests { + @Test func exactCountsUseTheExistingLabels() { + let presentation = LineChangeBadgePresentation( + addedLines: 12, + removedLines: 4, + skippedUntrackedFileCount: 0 + ) + + #expect(presentation.addedText == "+12") + #expect(presentation.removedText == "-4") + #expect(presentation.incompleteCountDescription == nil) + #expect(presentation.accessibilityLabel == "12 added lines, 4 removed lines") + } + + @Test func incompleteCountsRemainVisibleWithoutPretendingToBeExact() { + let presentation = LineChangeBadgePresentation( + addedLines: 0, + removedLines: 4, + skippedUntrackedFileCount: 1 + ) + + #expect(presentation.addedText == "+…") + #expect(presentation.removedText == "-4") + #expect( + presentation.incompleteCountDescription + == "Addition count is incomplete because 1 untracked file was not counted." + ) + #expect( + presentation.accessibilityLabel + == "Addition count incomplete, 0 lines counted; 1 untracked file was not counted. 4 removed lines." + ) + } + + @Test func incompleteNonzeroCountsShowTheirKnownLowerBound() { + let presentation = LineChangeBadgePresentation( + addedLines: 12, + removedLines: 4, + skippedUntrackedFileCount: 2 + ) + + #expect(presentation.addedText == "+12…") + #expect( + presentation.incompleteCountDescription + == "Addition count is incomplete because 2 untracked files were not counted." + ) + } +} diff --git a/supacodeTests/RepositoriesFeatureTests.swift b/supacodeTests/RepositoriesFeatureTests.swift index 65f6bf95..6ff18e10 100644 --- a/supacodeTests/RepositoriesFeatureTests.swift +++ b/supacodeTests/RepositoriesFeatureTests.swift @@ -196,8 +196,7 @@ struct RepositoriesFeatureTests { let changed = updateWorktreeLineChanges( worktreeID: worktree.id, - added: 12, - removed: 4, + changes: GitLineChanges(added: 12, removed: 4), state: &state ) @@ -221,8 +220,7 @@ struct RepositoriesFeatureTests { let changed = updateWorktreeLineChanges( worktreeID: worktree.id, - added: 0, - removed: 0, + changes: GitLineChanges(added: 0, removed: 0), state: &state ) @@ -240,8 +238,7 @@ struct RepositoriesFeatureTests { let changed = updateWorktreeLineChanges( worktreeID: worktree.id, - added: 12, - removed: 4, + changes: GitLineChanges(added: 12, removed: 4), state: &state ) @@ -252,6 +249,29 @@ struct RepositoriesFeatureTests { ) } + @Test func updateWorktreeLineChangesKeepsAnIncompleteZeroCountVisible() { + let worktree = makeWorktree(id: "/tmp/repo/feature", name: "feature", repoRoot: "/tmp/repo") + let repository = makeRepository(id: "/tmp/repo", worktrees: [worktree]) + var state = makeState(repositories: [repository]) + + let changed = updateWorktreeLineChanges( + worktreeID: worktree.id, + changes: GitLineChanges(added: 0, removed: 0, skippedUntrackedFileCount: 1), + state: &state + ) + + #expect(changed == true) + #expect( + state.worktreeInfoByID[worktree.id] + == WorktreeInfoEntry( + addedLines: 0, + removedLines: 0, + pullRequest: nil, + skippedUntrackedFileCount: 1 + ) + ) + } + @Test func filesChangedSkipsLineChangeActionWhenGitCountsMatchCurrentState() async { let worktree = makeWorktree(id: "/tmp/repo/feature", name: "feature", repoRoot: "/tmp/repo") let repository = makeRepository(id: "/tmp/repo", worktrees: [worktree]) @@ -265,7 +285,7 @@ struct RepositoriesFeatureTests { let store = TestStore(initialState: state) { RepositoriesFeature() } withDependencies: { - $0.gitClient.lineChanges = { _ in (12, 4) } + $0.gitClient.lineChanges = { _ in GitLineChanges(added: 12, removed: 4) } } await store.send(.worktreeInfoEvent(.filesChanged(worktreeID: worktree.id))) @@ -286,7 +306,7 @@ struct RepositoriesFeatureTests { let store = TestStore(initialState: state) { RepositoriesFeature() } withDependencies: { - $0.gitClient.lineChanges = { _ in (0, 0) } + $0.gitClient.lineChanges = { _ in GitLineChanges(added: 0, removed: 0) } } await store.send(.worktreeInfoEvent(.filesChanged(worktreeID: worktree.id))) @@ -306,7 +326,7 @@ struct RepositoriesFeatureTests { let store = TestStore(initialState: state) { RepositoriesFeature() } withDependencies: { - $0.gitClient.lineChanges = { _ in (0, 0) } + $0.gitClient.lineChanges = { _ in GitLineChanges(added: 0, removed: 0) } } await store.send(.worktreeInfoEvent(.filesChanged(worktreeID: worktree.id))) @@ -326,7 +346,7 @@ struct RepositoriesFeatureTests { let store = TestStore(initialState: state) { RepositoriesFeature() } withDependencies: { - $0.gitClient.lineChanges = { _ in (15, 9) } + $0.gitClient.lineChanges = { _ in GitLineChanges(added: 15, removed: 9) } } await store.send(.worktreeInfoEvent(.filesChanged(worktreeID: worktree.id))) @@ -339,6 +359,28 @@ struct RepositoriesFeatureTests { } } + @Test func filesChangedKeepsAnIncompleteLineCountVisible() async { + let worktree = makeWorktree(id: "/tmp/repo/feature", name: "feature", repoRoot: "/tmp/repo") + let repository = makeRepository(id: "/tmp/repo", worktrees: [worktree]) + let store = TestStore(initialState: makeState(repositories: [repository])) { + RepositoriesFeature() + } withDependencies: { + $0.gitClient.lineChanges = { _ in + GitLineChanges(added: 0, removed: 0, skippedUntrackedFileCount: 1) + } + } + + await store.send(.worktreeInfoEvent(.filesChanged(worktreeID: worktree.id))) + await store.receive(\.worktreeLineChangesLoaded) { + $0.worktreeInfoByID[worktree.id] = WorktreeInfoEntry( + addedLines: 0, + removedLines: 0, + pullRequest: nil, + skippedUntrackedFileCount: 1 + ) + } + } + @Test(.dependencies) func filesChangedSkipsLineChangesWhenObservationDisabled() async { let worktree = makeWorktree(id: "/tmp/repo/feature", name: "feature", repoRoot: "/tmp/repo") let repository = makeRepository(id: "/tmp/repo", worktrees: [worktree]) @@ -358,7 +400,7 @@ struct RepositoriesFeatureTests { } withDependencies: { $0.gitClient.lineChanges = { _ in lineChangeRequests.withValue { $0 += 1 } - return (15, 9) + return GitLineChanges(added: 15, removed: 9) } } @@ -390,6 +432,7 @@ struct RepositoriesFeatureTests { $0.snapshotPersistencePhase = .active } await store.receive(\.delegate.repositoriesChanged) + await store.finish() } @Test func plainRepositoryBecameGitRepositoryReloadsRepositories() async { @@ -5493,6 +5536,7 @@ struct RepositoriesFeatureTests { } await store.receive(\.delegate.repositoriesChanged) await store.receive(\.delegate.selectedWorktreeChanged) + await store.finish() } @Test func worktreeDeletedPrunesStateAndSendsDelegates() async { @@ -7544,9 +7588,17 @@ struct RepositoriesFeatureTests { applyWorkspaceChildrenInfo( [ WorkspaceChildInfoUpdate( - id: "/ws/app", branch: "feature", added: 7, removed: 2, pullRequest: pullRequest), + id: "/ws/app", + branch: "feature", + lineChanges: GitLineChanges(added: 7, removed: 2), + pullRequest: pullRequest + ), WorkspaceChildInfoUpdate( - id: "/ws/api", branch: " ", added: 0, removed: 0, pullRequest: nil), + id: "/ws/api", + branch: " ", + lineChanges: GitLineChanges(added: 0, removed: 0), + pullRequest: nil + ), ], state: &state ) @@ -7665,7 +7717,9 @@ struct RepositoriesFeatureTests { RepositoriesFeature() } withDependencies: { $0.gitClient.branchName = { _ in "feature/live" } - $0.gitClient.lineChanges = { _ in (7, 2) } + $0.gitClient.lineChanges = { _ in + GitLineChanges(added: 7, removed: 2, skippedUntrackedFileCount: 1) + } $0.repositoryPersistence.saveRepositorySnapshot = { _ in } } store.exhaustivity = .off @@ -7680,6 +7734,7 @@ struct RepositoriesFeatureTests { #expect(store.state.workspaceChildBranchByID[childID] == "feature/live") #expect(store.state.workspaceChildInfoByID[childID]?.addedLines == 7) #expect(store.state.workspaceChildInfoByID[childID]?.removedLines == 2) + #expect(store.state.workspaceChildInfoByID[childID]?.skippedUntrackedFileCount == 1) } @Test func openRepositoriesFinishedRefreshesWorkspaceChildren() async { @@ -7895,7 +7950,8 @@ struct RepositoriesFeatureTests { name: URL(fileURLWithPath: repoRootB).lastPathComponent, worktrees: [worktreeB] ) - let gate = AsyncGate() + let clock = TestClock() + let fetchStarted = AsyncGate() let startedRoots = LockIsolated>([]) let store = TestStore(initialState: RepositoriesFeature.State()) { @@ -7905,8 +7961,9 @@ struct RepositoriesFeatureTests { $0.gitClient.worktrees = { root in let path = root.path(percentEncoded: false) _ = startedRoots.withValue { $0.insert(path) } + await fetchStarted.resume() if path == repoRootA { - await gate.wait() + try? await clock.sleep(for: .seconds(1)) return [worktreeA] } if path == repoRootB { @@ -7918,18 +7975,10 @@ struct RepositoriesFeatureTests { } await store.send(.loadPersistedRepositories) - - var secondFetchStarted = false - for _ in 0..<100 { - if startedRoots.value.contains(repoRootB) { - secondFetchStarted = true - break - } - await Task.yield() - } - #expect(secondFetchStarted) - - await gate.resume() + await fetchStarted.wait() + await clock.advance() + #expect(startedRoots.value == [repoRootA, repoRootB]) + await clock.advance(by: .seconds(1)) await store.receive(\.repositoriesLoaded) { $0.repositories = [repoA, repoB] -- 2.51.2 From 925449f9b8de1e8e90f6afd6797edcd51cccbabd Mon Sep 17 00:00:00 2001 From: onevcat Date: Sun, 2 Aug 2026 00:29:10 +0900 Subject: [PATCH 15/31] Link performance follow-up record --- docs-ai/056-performance-optimization-2026-08/000-plan.md | 2 +- docs-ai/056-performance-optimization-2026-08/001-action.md | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/docs-ai/056-performance-optimization-2026-08/000-plan.md b/docs-ai/056-performance-optimization-2026-08/000-plan.md index b962c021..054abd38 100644 --- a/docs-ai/056-performance-optimization-2026-08/000-plan.md +++ b/docs-ai/056-performance-optimization-2026-08/000-plan.md @@ -4,7 +4,7 @@ | --- | --- | | **Status** | Implemented | | **Anchor date** | 2026-08-01 | -| **Primary PRs** | #644 and its fork follow-up; review queue #645–#650 | +| **Primary PRs** | #644, #652; review queue #645–#650 | | **Related** | [032-performance-hardening](../032-performance-hardening/000-plan.md), [037-line-diff-tracking](../037-line-diff-tracking/000-plan.md), `docs/components/diff-view.md` | ## Background diff --git a/docs-ai/056-performance-optimization-2026-08/001-action.md b/docs-ai/056-performance-optimization-2026-08/001-action.md index 818f4ac1..ea83e1da 100644 --- a/docs-ai/056-performance-optimization-2026-08/001-action.md +++ b/docs-ai/056-performance-optimization-2026-08/001-action.md @@ -4,7 +4,7 @@ | --- | --- | | **Status** | Implemented | | **Anchor date** | 2026-08-01 | -| **Primary PRs** | #644; follow-up PR from `perf/untracked-line-count-completion` | +| **Primary PRs** | #644, #652 | ## Outcome -- 2.51.2 From c62ae04960defdbee80b29b77d0a16963af434d7 Mon Sep 17 00:00:00 2001 From: onevcat Date: Sun, 2 Aug 2026 01:07:32 +0900 Subject: [PATCH 16/31] Bound untracked line-count cache retention --- .../000-plan.md | 17 +- .../001-action.md | 9 +- docs/components/diff-view.md | 13 +- supacode/Clients/Git/GitClient.swift | 34 ++-- .../Clients/Git/UntrackedLineCountCache.swift | 188 ++++++++++++++++-- supacodeTests/GitClientLineChangesTests.swift | 71 +++++++ 6 files changed, 288 insertions(+), 44 deletions(-) diff --git a/docs-ai/056-performance-optimization-2026-08/000-plan.md b/docs-ai/056-performance-optimization-2026-08/000-plan.md index 054abd38..73403a6b 100644 --- a/docs-ai/056-performance-optimization-2026-08/000-plan.md +++ b/docs-ai/056-performance-optimization-2026-08/000-plan.md @@ -56,8 +56,8 @@ bounding this dense-match case. - Preserve #644's original author commits and measurable scan improvement. - Remove the silent per-file 2 MiB semantic cutoff. -- Cache exact untracked-file line counts across refreshes using stable file metadata, so an - unchanged large capture is not reread when another file triggers FSEvents. +- Cache calculated untracked-file line counts across refreshes using stable file metadata, so a + metadata-unchanged large capture is not reread when another file triggers FSEvents. - Bound uncached work with one deterministic byte budget for the whole refresh, not a per-file limit that can still scan an unbounded aggregate. - Propagate incomplete-count state to the sidebar and workspace-child rows instead of @@ -78,17 +78,18 @@ bounding this dense-match case. ## Design / Approach -### Exact cache +### Metadata-validated cache Introduce a long-lived, concurrency-safe untracked-line cache shared by live `GitClient` instances. Entries are scoped by worktree and relative path and fingerprinted with file -identity, byte size, and modification date. An unchanged fingerprint reuses either the -exact text line count or the binary-file result. Every current `git ls-files --others` -result prunes disappeared paths from that worktree's cache. +identity, byte size, and modification date. A metadata-equivalent fingerprint reuses either +the previously calculated text line count or the binary-file result. Every current +`git ls-files --others` result prunes disappeared paths from that worktree's cache. Cache misses are scanned outside cache isolation so independent worktrees are not forced -through one serialized I/O executor. Updates are committed with their fingerprint; a -concurrent stale result cannot be reused after the file metadata changes. +through one serialized I/O executor. Updates are committed with their fingerprint, and a +later refresh whose metadata differs will not reuse the cached result. Cache retention is +bounded by worktree, entry, and relative-path-key limits. ### Aggregate scan budget diff --git a/docs-ai/056-performance-optimization-2026-08/001-action.md b/docs-ai/056-performance-optimization-2026-08/001-action.md index ea83e1da..0728c687 100644 --- a/docs-ai/056-performance-optimization-2026-08/001-action.md +++ b/docs-ai/056-performance-optimization-2026-08/001-action.md @@ -10,7 +10,7 @@ The follow-up preserves both commits from #644 and keeps its `memchr` speedup, but removes the silent 2 MiB per-file cutoff. Untracked-file line counts now use -an exact metadata cache plus one 32 MiB budget across uncached files for a +a metadata-validated cache plus one 32 MiB budget across uncached files for a refresh. Cached files cost no scan budget, while files that do not fit are reported explicitly instead of being folded into an exact zero. @@ -28,10 +28,11 @@ files. plus the number of omitted files. Its sparse `memchr` scan switches to a raw pointer loop after 2,048 matches in one 64 KiB chunk to bound newline-dense input. -- `UntrackedLineCountCache.swift` — stores exact text counts and binary results +- `UntrackedLineCountCache.swift` — stores counted text values and binary results by worktree/path and file identity, size, and modification date. It prunes - disappeared paths and retains at most 128 worktree roots with LRU eviction. - File I/O stays outside the short cache lock. + disappeared paths, retains at most 128 worktree roots, and caps both cache + entries and retained relative-path bytes with LRU eviction. File I/O stays + outside the short cache lock. - `GitClientTypes.swift` and the repository reducer/state files — carry one structured `GitLineChanges` value through regular worktrees and project workspace children, including the omitted-untracked-file count. diff --git a/docs/components/diff-view.md b/docs/components/diff-view.md index 8d6cf187..0b770c51 100644 --- a/docs/components/diff-view.md +++ b/docs/components/diff-view.md @@ -94,12 +94,13 @@ Repositories can show **line-change badges** (additions/deletions) on worktree rows, controlled per repo by `observeLineDiffsAutomatically` (on by default). Disable it for very large repos if it's expensive. -Prowl caches line counts for unchanged untracked files. On a cold refresh it -scans at most 32 MiB of uncached untracked content across the worktree. If more -content remains, the additions label ends in an ellipsis (`+N…`, or `+…` when -no additions were counted yet), and its tooltip identifies how many untracked -files were omitted. The badge stays available to open Show Diff, which still -lists every changed file. Tracked additions and deletions remain exact. +Prowl caches line counts for untracked files whose metadata has not changed. +The cache has bounded entry and path-key storage. On a cold refresh it scans at +most 32 MiB of uncached untracked content across the worktree. If more content +remains, the additions label ends in an ellipsis (`+N…`, or `+…` when no +additions were counted yet), and its tooltip identifies how many untracked files +were omitted. The badge stays available to open Show Diff, which still lists +every changed file. Tracked additions and deletions remain exact. ## Availability diff --git a/supacode/Clients/Git/GitClient.swift b/supacode/Clients/Git/GitClient.swift index 2a1a17aa..9dc3b02d 100644 --- a/supacode/Clients/Git/GitClient.swift +++ b/supacode/Clients/Git/GitClient.swift @@ -463,6 +463,7 @@ struct GitClient { } nonisolated static let untrackedLineCountByteBudget = 32 * 1_024 * 1_024 + nonisolated static let untrackedLineCountCacheUpdateBatchSize = 256 nonisolated static func countLinesInFiles( _ relativePaths: [String], @@ -524,6 +525,7 @@ struct GitClient { var remainingByteBudget = max(0, byteBudget) var cacheUpdates: [UntrackedLineCacheUpdate] = [] + cacheUpdates.reserveCapacity(untrackedLineCountCacheUpdateBatchSize) for file in misses.sorted(by: { lhs, rhs in if lhs.fingerprint.byteCount == rhs.fingerprint.byteCount { return lhs.relativePath < rhs.relativePath @@ -534,30 +536,38 @@ struct GitClient { skippedFileCount += 1 continue } + let cacheUpdate: UntrackedLineCacheUpdate? switch Self.countLines(in: file.url, maximumByteCount: remainingByteBudget) { case .text(let lines, let bytesRead): total += lines remainingByteBudget -= bytesRead - cacheUpdates.append( - UntrackedLineCacheUpdate( - relativePath: file.relativePath, - fingerprint: file.fingerprint, - value: .text(lines) - )) + cacheUpdate = UntrackedLineCacheUpdate( + relativePath: file.relativePath, + fingerprint: file.fingerprint, + value: .text(lines) + ) case .binary(let bytesRead): remainingByteBudget -= bytesRead - cacheUpdates.append( - UntrackedLineCacheUpdate( - relativePath: file.relativePath, - fingerprint: file.fingerprint, - value: .binary - )) + cacheUpdate = UntrackedLineCacheUpdate( + relativePath: file.relativePath, + fingerprint: file.fingerprint, + value: .binary + ) case .budgetExceeded: remainingByteBudget = 0 skippedFileCount += 1 + cacheUpdate = nil case .unavailable(let bytesRead): remainingByteBudget -= bytesRead skippedFileCount += 1 + cacheUpdate = nil + } + if let cacheUpdate { + cacheUpdates.append(cacheUpdate) + if cacheUpdates.count == untrackedLineCountCacheUpdateBatchSize { + cache.store(cacheUpdates, worktreeKey: worktreeKey) + cacheUpdates.removeAll(keepingCapacity: true) + } } } cache.store(cacheUpdates, worktreeKey: worktreeKey) diff --git a/supacode/Clients/Git/UntrackedLineCountCache.swift b/supacode/Clients/Git/UntrackedLineCountCache.swift index 07a61bb9..491ae9f3 100644 --- a/supacode/Clients/Git/UntrackedLineCountCache.swift +++ b/supacode/Clients/Git/UntrackedLineCountCache.swift @@ -29,20 +29,51 @@ nonisolated final class UntrackedLineCountCache: Sendable { private struct Entry { let fingerprint: UntrackedLineFileFingerprint let value: CachedUntrackedLineCount + let cachedPathByteCount: Int + var accessSequence: UInt64 + } + + private struct EntryIdentifier { + let worktreeKey: String + let relativePath: String + } + + private struct EntryCandidate { + let identifier: EntryIdentifier + let entry: Entry } private struct State { let maximumWorktreeCount: Int + let maximumEntryCountPerWorktree: Int + let maximumTotalEntryCount: Int + let maximumCachedPathByteCount: Int var entriesByWorktree: [String: [String: Entry]] = [:] var lastAccessByWorktree: [String: UInt64] = [:] + var cachedEntryCount = 0 + var cachedPathByteCount = 0 var accessSequence: UInt64 = 0 } private let state: Mutex - init(maximumWorktreeCount: Int = 128) { + init( + maximumWorktreeCount: Int = 128, + maximumEntryCountPerWorktree: Int = 4_096, + maximumTotalEntryCount: Int = 32_768, + maximumCachedPathByteCount: Int = 4 * 1_024 * 1_024 + ) { precondition(maximumWorktreeCount > 0) - state = Mutex(State(maximumWorktreeCount: maximumWorktreeCount)) + precondition(maximumEntryCountPerWorktree > 0) + precondition(maximumTotalEntryCount > 0) + precondition(maximumCachedPathByteCount > 0) + state = Mutex( + State( + maximumWorktreeCount: maximumWorktreeCount, + maximumEntryCountPerWorktree: maximumEntryCountPerWorktree, + maximumTotalEntryCount: maximumTotalEntryCount, + maximumCachedPathByteCount: maximumCachedPathByteCount + )) } func cachedValues( @@ -51,24 +82,36 @@ nonisolated final class UntrackedLineCountCache: Sendable { ) -> [String: CachedUntrackedLineCount] { state.withLock { state in guard !files.isEmpty else { - state.entriesByWorktree.removeValue(forKey: worktreeKey) - state.lastAccessByWorktree.removeValue(forKey: worktreeKey) + removeWorktree(worktreeKey, state: &state) return [:] } let currentPaths = Set(files.map(\.relativePath)) var entries = state.entriesByWorktree[worktreeKey, default: [:]] - entries = entries.filter { currentPaths.contains($0.key) } - state.entriesByWorktree[worktreeKey] = entries - touch(worktreeKey, state: &state) - trimIfNeeded(state: &state) + for relativePath in Array(entries.keys) where !currentPaths.contains(relativePath) { + remove(relativePath, from: &entries, state: &state) + } var result: [String: CachedUntrackedLineCount] = [:] for file in files { - guard let entry = entries[file.relativePath], entry.fingerprint == file.fingerprint else { + guard var entry = entries[file.relativePath] else { continue } + guard entry.fingerprint == file.fingerprint else { + remove(file.relativePath, from: &entries, state: &state) + continue + } + entry.accessSequence = nextAccess(state: &state) + entries[file.relativePath] = entry result[file.relativePath] = entry.value } + if entries.isEmpty { + state.entriesByWorktree.removeValue(forKey: worktreeKey) + state.lastAccessByWorktree.removeValue(forKey: worktreeKey) + } else { + state.entriesByWorktree[worktreeKey] = entries + touch(worktreeKey, state: &state) + trimIfNeeded(state: &state) + } return result } } @@ -81,10 +124,18 @@ nonisolated final class UntrackedLineCountCache: Sendable { state.withLock { state in var entries = state.entriesByWorktree[worktreeKey, default: [:]] for update in updates { - entries[update.relativePath] = Entry( + let entry = Entry( fingerprint: update.fingerprint, - value: update.value + value: update.value, + cachedPathByteCount: update.relativePath.utf8.count, + accessSequence: nextAccess(state: &state) ) + if let replaced = entries.updateValue(entry, forKey: update.relativePath) { + state.cachedPathByteCount += entry.cachedPathByteCount - replaced.cachedPathByteCount + } else { + state.cachedEntryCount += 1 + state.cachedPathByteCount += entry.cachedPathByteCount + } } state.entriesByWorktree[worktreeKey] = entries touch(worktreeKey, state: &state) @@ -93,16 +144,125 @@ nonisolated final class UntrackedLineCountCache: Sendable { } private func touch(_ worktreeKey: String, state: inout State) { + state.lastAccessByWorktree[worktreeKey] = nextAccess(state: &state) + } + + private func nextAccess(state: inout State) -> UInt64 { state.accessSequence &+= 1 - state.lastAccessByWorktree[worktreeKey] = state.accessSequence + return state.accessSequence } private func trimIfNeeded(state: inout State) { while state.entriesByWorktree.count > state.maximumWorktreeCount, let leastRecentlyUsed = state.lastAccessByWorktree.min(by: { $0.value < $1.value })?.key { - state.entriesByWorktree.removeValue(forKey: leastRecentlyUsed) - state.lastAccessByWorktree.removeValue(forKey: leastRecentlyUsed) + removeWorktree(leastRecentlyUsed, state: &state) + } + + for worktreeKey in Array(state.entriesByWorktree.keys) { + trimEntries(in: worktreeKey, state: &state) + } + + while state.cachedEntryCount > state.maximumTotalEntryCount + || state.cachedPathByteCount > state.maximumCachedPathByteCount + { + guard let leastRecentlyUsed = leastRecentlyUsedEntry(in: state) else { + break + } + remove(leastRecentlyUsed, state: &state) + } + + } + + private func trimEntries(in worktreeKey: String, state: inout State) { + guard var entries = state.entriesByWorktree[worktreeKey] else { + return + } + while entries.count > state.maximumEntryCountPerWorktree, + let leastRecentlyUsed = leastRecentlyUsedEntry(in: entries) + { + remove(leastRecentlyUsed, from: &entries, state: &state) + } + if entries.isEmpty { + state.entriesByWorktree.removeValue(forKey: worktreeKey) + state.lastAccessByWorktree.removeValue(forKey: worktreeKey) + } else { + state.entriesByWorktree[worktreeKey] = entries + } + } + + private func leastRecentlyUsedEntry(in entries: [String: Entry]) -> String? { + entries.min { lhs, rhs in + if lhs.value.accessSequence != rhs.value.accessSequence { + return lhs.value.accessSequence < rhs.value.accessSequence + } + return lhs.key < rhs.key + }?.key + } + + private func leastRecentlyUsedEntry(in state: State) -> EntryIdentifier? { + var result: EntryCandidate? + for (worktreeKey, entries) in state.entriesByWorktree { + for (relativePath, entry) in entries { + guard let current = result else { + result = EntryCandidate( + identifier: EntryIdentifier(worktreeKey: worktreeKey, relativePath: relativePath), + entry: entry + ) + continue + } + if entry.accessSequence < current.entry.accessSequence + || (entry.accessSequence == current.entry.accessSequence + && (worktreeKey < current.identifier.worktreeKey + || (worktreeKey == current.identifier.worktreeKey + && relativePath < current.identifier.relativePath))) + { + result = EntryCandidate( + identifier: EntryIdentifier(worktreeKey: worktreeKey, relativePath: relativePath), + entry: entry + ) + } + } + } + return result?.identifier + } + + private func remove( + _ entry: EntryIdentifier, + state: inout State + ) { + guard var entries = state.entriesByWorktree[entry.worktreeKey] else { + return + } + remove(entry.relativePath, from: &entries, state: &state) + if entries.isEmpty { + state.entriesByWorktree.removeValue(forKey: entry.worktreeKey) + state.lastAccessByWorktree.removeValue(forKey: entry.worktreeKey) + } else { + state.entriesByWorktree[entry.worktreeKey] = entries + } + } + + private func remove( + _ relativePath: String, + from entries: inout [String: Entry], + state: inout State + ) { + guard let removed = entries.removeValue(forKey: relativePath) else { + return + } + state.cachedEntryCount -= 1 + state.cachedPathByteCount -= removed.cachedPathByteCount + } + + private func removeWorktree(_ worktreeKey: String, state: inout State) { + guard let entries = state.entriesByWorktree.removeValue(forKey: worktreeKey) else { + return + } + state.cachedEntryCount -= entries.count + state.cachedPathByteCount -= entries.values.reduce(into: 0) { count, entry in + count += entry.cachedPathByteCount } + state.lastAccessByWorktree.removeValue(forKey: worktreeKey) } } diff --git a/supacodeTests/GitClientLineChangesTests.swift b/supacodeTests/GitClientLineChangesTests.swift index 857d71dc..f9424c38 100644 --- a/supacodeTests/GitClientLineChangesTests.swift +++ b/supacodeTests/GitClientLineChangesTests.swift @@ -496,6 +496,77 @@ struct GitClientLineChangesTests { #expect(evicted == UntrackedLineCountResult(lines: 0, skippedFileCount: 1)) } + @Test func countLinesInFilesBoundsCachedEntriesPerWorktree() throws { + let fileManager = FileManager.default + let tempRoot = fileManager.temporaryDirectory.appending(path: UUID().uuidString) + defer { try? fileManager.removeItem(at: tempRoot) } + try fileManager.createDirectory(at: tempRoot, withIntermediateDirectories: true) + try "a\n".write(to: tempRoot.appending(path: "a.txt"), atomically: true, encoding: .utf8) + try "b\n".write(to: tempRoot.appending(path: "b.txt"), atomically: true, encoding: .utf8) + let cache = UntrackedLineCountCache( + maximumWorktreeCount: 1, + maximumEntryCountPerWorktree: 1, + maximumTotalEntryCount: 2, + maximumCachedPathByteCount: 128 + ) + + _ = GitClient.countLinesInFiles( + ["a.txt", "b.txt"], relativeTo: tempRoot, cache: cache, byteBudget: 4) + let capped = GitClient.countLinesInFiles( + ["a.txt", "b.txt"], relativeTo: tempRoot, cache: cache, byteBudget: 0) + + #expect(capped == UntrackedLineCountResult(lines: 1, skippedFileCount: 1)) + } + + @Test func countLinesInFilesBoundsCachedEntriesAcrossWorktrees() throws { + let fileManager = FileManager.default + let tempRoot = fileManager.temporaryDirectory.appending(path: UUID().uuidString) + defer { try? fileManager.removeItem(at: tempRoot) } + let firstRoot = tempRoot.appending(path: "first", directoryHint: .isDirectory) + let secondRoot = tempRoot.appending(path: "second", directoryHint: .isDirectory) + try fileManager.createDirectory(at: firstRoot, withIntermediateDirectories: true) + try fileManager.createDirectory(at: secondRoot, withIntermediateDirectories: true) + try "a\n".write(to: firstRoot.appending(path: "file.txt"), atomically: true, encoding: .utf8) + try "b\n".write(to: secondRoot.appending(path: "file.txt"), atomically: true, encoding: .utf8) + let cache = UntrackedLineCountCache( + maximumWorktreeCount: 2, + maximumEntryCountPerWorktree: 2, + maximumTotalEntryCount: 1, + maximumCachedPathByteCount: 128 + ) + + _ = GitClient.countLinesInFiles( + ["file.txt"], relativeTo: firstRoot, cache: cache, byteBudget: 2) + _ = GitClient.countLinesInFiles( + ["file.txt"], relativeTo: secondRoot, cache: cache, byteBudget: 2) + let capped = GitClient.countLinesInFiles( + ["file.txt"], relativeTo: firstRoot, cache: cache, byteBudget: 0) + + #expect(capped == UntrackedLineCountResult(lines: 0, skippedFileCount: 1)) + } + + @Test func countLinesInFilesBoundsCachedPathBytes() throws { + let fileManager = FileManager.default + let tempRoot = fileManager.temporaryDirectory.appending(path: UUID().uuidString) + defer { try? fileManager.removeItem(at: tempRoot) } + try fileManager.createDirectory(at: tempRoot, withIntermediateDirectories: true) + try "a\n".write(to: tempRoot.appending(path: "a.txt"), atomically: true, encoding: .utf8) + try "b\n".write(to: tempRoot.appending(path: "b.txt"), atomically: true, encoding: .utf8) + let cache = UntrackedLineCountCache( + maximumWorktreeCount: 1, + maximumEntryCountPerWorktree: 2, + maximumTotalEntryCount: 2, + maximumCachedPathByteCount: 9 + ) + + _ = GitClient.countLinesInFiles( + ["a.txt", "b.txt"], relativeTo: tempRoot, cache: cache, byteBudget: 4) + let capped = GitClient.countLinesInFiles( + ["a.txt", "b.txt"], relativeTo: tempRoot, cache: cache, byteBudget: 0) + + #expect(capped == UntrackedLineCountResult(lines: 1, skippedFileCount: 1)) + } + @Test func countLinesInFilesSupportsConcurrentRefreshes() async throws { let fileManager = FileManager.default let tempRoot = fileManager.temporaryDirectory.appending(path: UUID().uuidString) -- 2.51.2 From 17f94b656ba9fb0ca4429bbed31b139e3c7e4a80 Mon Sep 17 00:00:00 2001 From: onevcat Date: Sun, 2 Aug 2026 01:38:02 +0900 Subject: [PATCH 17/31] Document opt-in TCA action logging review --- .../000-plan.md | 26 ++++++--- .../002-opt-in-debug-tca-action-logging.md | 55 +++++++++++++++++++ docs-ai/README.md | 2 +- 3 files changed, 73 insertions(+), 10 deletions(-) create mode 100644 docs-ai/056-performance-optimization-2026-08/002-opt-in-debug-tca-action-logging.md diff --git a/docs-ai/056-performance-optimization-2026-08/000-plan.md b/docs-ai/056-performance-optimization-2026-08/000-plan.md index 73403a6b..4369bb9b 100644 --- a/docs-ai/056-performance-optimization-2026-08/000-plan.md +++ b/docs-ai/056-performance-optimization-2026-08/000-plan.md @@ -4,7 +4,7 @@ | --- | --- | | **Status** | Implemented | | **Anchor date** | 2026-08-01 | -| **Primary PRs** | #644, #652; review queue #645–#650 | +| **Primary PRs** | #644, #645, #652; review queue #646–#650 | | **Related** | [032-performance-hardening](../032-performance-hardening/000-plan.md), [037-line-diff-tracking](../037-line-diff-tracking/000-plan.md), `docs/components/diff-view.md` | ## Background @@ -25,11 +25,16 @@ This entry records the cross-cutting standard used to review and integrate that - require tests that can fail when the optimized path stops being equivalent; - keep each PR independently reviewable instead of combining unrelated hot paths. -The first application is #644. Its `memchr` scan removes the dominant per-byte +The first application was #644. Its `memchr` scan removes the dominant per-byte `Data.Iterator` overhead, but its per-file 2 MiB cutoff silently maps a present, readable untracked text file to zero added lines. If that is the only change, the worktree badge disappears even though Show Diff still includes the file. +The second application is #645. The root reducer remains wrapped for opt-in diagnostics, +but a normal Debug launch now bypasses action reflection, state snapshots and equality +checks, and `CustomDump` diff generation. See +[056.002](002-opt-in-debug-tca-action-logging.md) for the reviewed behavior and scope. + ## Measured baseline for #644 An independent Release-optimized benchmark on an Apple M2 Pro mirrored the production @@ -71,10 +76,10 @@ bounding this dense-match case. - No user-facing setting for byte budgets or cache policy in this wave. - No replacement of `git diff HEAD --shortstat` or the FSEvents scheduling pipeline. -- No combined implementation of #645–#650; each remains an independent review and merge +- No combined implementation of #646–#650; each remains an independent review and merge decision. -- No claim that author-reported sampling numbers are independently verified until that PR - receives its own review. +- No author-reported sampling number is treated as independently verified without a + same-path reproduction. ## Design / Approach @@ -117,13 +122,13 @@ input while preserving exact counts. ## Performance PR review queue -The descriptions and heads below were confirmed on 2026-08-01. All claims remain pending -independent code-path and test review except #644, which is the active implementation wave. +The descriptions and heads below were confirmed on 2026-08-02. Claims for #646–#650 remain +pending independent code-path and test review. | PR | Confirmed scope | Review focus / placeholder | Head | | --- | --- | --- | --- | -| #644 | Replace `Data.Iterator` line scans and avoid repeated large untracked-file work | Active: preserve exact badge semantics with cache, aggregate budget, and explicit incomplete state | `978b7b59` | -| #645 | Gate Debug TCA action reflection/state-diff logging behind `PROWL_LOG_TCA_ACTIONS` | Verify flag parsing, logging contract, Debug-only scope, and pass-through reducer semantics | `616bbf4b` | +| #644 | Replace `Data.Iterator` line scans and avoid repeated large untracked-file work | Integrated through #652 with exact cached counts, a refresh-wide budget, and explicit incomplete state | `978b7b59` | +| #645 | Gate Debug TCA action reflection/state-diff logging behind `PROWL_LOG_TCA_ACTIONS` | Reviewed: default Debug launches bypass the expensive diagnostics; the opt-in path remains available and Release behavior is unchanged | `616bbf4b` | | #646 | Memoize per-surface agent screen parsing when agent and visible text are unchanged | Verify cache invalidation, stabilization timing, observation isolation, and surface teardown | `b2ac2936` | | #647 | Deduplicate raw-state-only agent emissions and narrow sidebar invalidation | Decide whether stale CLI `raw_state` is acceptable; trace UI/CLI ownership before merge | `e77ba660` | | #648 | Replace per-agent worktree scans/path resolution with a cached directory index | Verify deepest-match and symlink semantics, cache invalidation, render-path purity, and current CI | `08773383` | @@ -157,3 +162,6 @@ independent code-path and test review except #644, which is the active implement hybrid scanner change. ## Amendments + +- Updated 2026-08-02: Reviewed and integrated opt-in Debug TCA action logging from #645 — see + [002-opt-in-debug-tca-action-logging.md](002-opt-in-debug-tca-action-logging.md). diff --git a/docs-ai/056-performance-optimization-2026-08/002-opt-in-debug-tca-action-logging.md b/docs-ai/056-performance-optimization-2026-08/002-opt-in-debug-tca-action-logging.md new file mode 100644 index 00000000..4fe9fc24 --- /dev/null +++ b/docs-ai/056-performance-optimization-2026-08/002-opt-in-debug-tca-action-logging.md @@ -0,0 +1,55 @@ +# 056.002 — Opt-in Debug TCA Action Logging + +## Context + +The root `AppFeature` reducer is wrapped by `LogActionsReducer` in +`supacode/App/supacodeApp.swift`. Before #645, every action in a Debug build paid for +`debugCaseOutput` reflection, a pre-reduction state snapshot, full-state equality, and a +`CustomDump` diff when state changed. That diagnostic path ran even when its stdout was not +observable from a Finder- or launchd-launched app. + +The author reported that a sampled idle workload with 24 terminal surfaces attributed about +5% of main-thread time to this path, with the wrapper accounting for 93% of sampled +main-thread reducer time. Those percentages are author measurements; the review verified +the code path and resulting bypass, not the original capture. + +## Change + +- PR #645 adds the launch-scoped `PROWL_LOG_TCA_ACTIONS=1` gate in + `supacode/Support/DebugCaseOutput.swift`. +- With the flag absent or set to any other value, the Debug wrapper immediately calls + `base.reduce` and skips action reflection, state snapshotting and comparison, and diff + generation. +- With the flag set, action labels use `SupaLogger.notice` so they reach the unified log and + `make log-stream`; state diffs remain on stdout through `print`. +- `supacode/Support/SupaLogger.swift` now owns an `OSLog.Logger` in Debug as well as Release + and exposes `notice` for diagnostics that must reach the unified log in both configurations. +- The Release branch is unchanged: it still emits the existing compact action label, + Sentry log entry, and breadcrumb before reducing the action. +- `AGENTS.md` documents the opt-in flag and its default-off behavior. + +## Refs + +- PR #645 +- Original implementation commit `616bbf4b` + +## Current state + +The default Debug hot path now adds only the flag branch and the direct call to the wrapped +reducer; the measured reflection, state-copy/equality, and diff costs are not reachable. The +diagnostic behavior remains intentionally expensive when explicitly enabled. + +The focused tests in `supacodeTests/LogActionsReducerTests.swift` cover state mutation through +the wrapper, but they do not distinguish enabled from disabled flag evaluation and their +test reducer returns no effects. The implementation's pass-through is direct and low risk, +but future changes to the gate should use an injectable configuration seam if deterministic +branch and effect-forwarding coverage becomes important. + +## Verification + +- `LogActionsReducerTests` passed 2 tests with `PROWL_LOG_TCA_ACTIONS` absent. +- The same focused suite passed 2 tests with `PROWL_LOG_TCA_ACTIONS=1`. +- `make check` passed. +- `make build-app` completed with no errors or warnings. +- The author-reported sampled percentages were not independently reproduced during this + review. diff --git a/docs-ai/README.md b/docs-ai/README.md index cd857c72..3d44b406 100644 --- a/docs-ai/README.md +++ b/docs-ai/README.md @@ -106,4 +106,4 @@ agent-facing manual for that). | 052 | [sidebar-context-menus](052-sidebar-context-menus/000-plan.md) | 2026-07-27 | Sidebar context menu overhaul: worktree terminal actions, header/workspace path actions, PR click-through | | 053 | [agent-profiles](053-agent-profiles/000-plan.md) | 2026-07-29 | Agent Profile V1: preset-first Claude Code/Codex launch profiles, opt-in account isolation, path routing, and future handoff seam | | 054 | [native-settings-navigation](054-native-settings-navigation/000-plan.md) | 2026-07-30 | SwiftUI-owned singleton Settings window, native sidebar/detail navigation, and Agent Profile drill-ins | -| 056 | [performance-optimization-2026-08](056-performance-optimization-2026-08/000-plan.md) | 2026-08-01 | Review and integration standard for the August performance series; exact cached and bounded untracked-line counts | +| 056 | [performance-optimization-2026-08](056-performance-optimization-2026-08/000-plan.md) | 2026-08-01 | August performance review series: exact cached line counts and opt-in Debug TCA action logging | -- 2.51.2 From e951c42e910541ac93cc09a915cd836ffcc4087a Mon Sep 17 00:00:00 2001 From: onevcat Date: Sun, 2 Aug 2026 01:41:50 +0900 Subject: [PATCH 18/31] Link TCA logging integration PR --- .../000-plan.md | 13 +++++++------ .../002-opt-in-debug-tca-action-logging.md | 3 ++- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/docs-ai/056-performance-optimization-2026-08/000-plan.md b/docs-ai/056-performance-optimization-2026-08/000-plan.md index 4369bb9b..719eba90 100644 --- a/docs-ai/056-performance-optimization-2026-08/000-plan.md +++ b/docs-ai/056-performance-optimization-2026-08/000-plan.md @@ -4,7 +4,7 @@ | --- | --- | | **Status** | Implemented | | **Anchor date** | 2026-08-01 | -| **Primary PRs** | #644, #645, #652; review queue #646–#650 | +| **Primary PRs** | #644, #645, #652, #653; review queue #646–#650 | | **Related** | [032-performance-hardening](../032-performance-hardening/000-plan.md), [037-line-diff-tracking](../037-line-diff-tracking/000-plan.md), `docs/components/diff-view.md` | ## Background @@ -30,9 +30,9 @@ The first application was #644. Its `memchr` scan removes the dominant per-byte untracked text file to zero added lines. If that is the only change, the worktree badge disappears even though Show Diff still includes the file. -The second application is #645. The root reducer remains wrapped for opt-in diagnostics, -but a normal Debug launch now bypasses action reflection, state snapshots and equality -checks, and `CustomDump` diff generation. See +The second application originated in #645 and is integrated through #653. The root reducer +remains wrapped for opt-in diagnostics, but a normal Debug launch now bypasses action +reflection, state snapshots and equality checks, and `CustomDump` diff generation. See [056.002](002-opt-in-debug-tca-action-logging.md) for the reviewed behavior and scope. ## Measured baseline for #644 @@ -128,7 +128,7 @@ pending independent code-path and test review. | PR | Confirmed scope | Review focus / placeholder | Head | | --- | --- | --- | --- | | #644 | Replace `Data.Iterator` line scans and avoid repeated large untracked-file work | Integrated through #652 with exact cached counts, a refresh-wide budget, and explicit incomplete state | `978b7b59` | -| #645 | Gate Debug TCA action reflection/state-diff logging behind `PROWL_LOG_TCA_ACTIONS` | Reviewed: default Debug launches bypass the expensive diagnostics; the opt-in path remains available and Release behavior is unchanged | `616bbf4b` | +| #645 | Gate Debug TCA action reflection/state-diff logging behind `PROWL_LOG_TCA_ACTIONS` | Reviewed and integrated through #653: default Debug launches bypass the expensive diagnostics; the opt-in path remains available and Release behavior is unchanged | `616bbf4b` | | #646 | Memoize per-surface agent screen parsing when agent and visible text are unchanged | Verify cache invalidation, stabilization timing, observation isolation, and surface teardown | `b2ac2936` | | #647 | Deduplicate raw-state-only agent emissions and narrow sidebar invalidation | Decide whether stale CLI `raw_state` is acceptable; trace UI/CLI ownership before merge | `e77ba660` | | #648 | Replace per-agent worktree scans/path resolution with a cached directory index | Verify deepest-match and symlink semantics, cache invalidation, render-path purity, and current CI | `08773383` | @@ -163,5 +163,6 @@ pending independent code-path and test review. ## Amendments -- Updated 2026-08-02: Reviewed and integrated opt-in Debug TCA action logging from #645 — see +- Updated 2026-08-02: Reviewed opt-in Debug TCA action logging from #645 and moved integration + to fork-owned PR #653 — see [002-opt-in-debug-tca-action-logging.md](002-opt-in-debug-tca-action-logging.md). diff --git a/docs-ai/056-performance-optimization-2026-08/002-opt-in-debug-tca-action-logging.md b/docs-ai/056-performance-optimization-2026-08/002-opt-in-debug-tca-action-logging.md index 4fe9fc24..3a24d2e0 100644 --- a/docs-ai/056-performance-optimization-2026-08/002-opt-in-debug-tca-action-logging.md +++ b/docs-ai/056-performance-optimization-2026-08/002-opt-in-debug-tca-action-logging.md @@ -30,7 +30,8 @@ the code path and resulting bypass, not the original capture. ## Refs -- PR #645 +- Original PR #645 +- Fork integration PR #653 - Original implementation commit `616bbf4b` ## Current state -- 2.51.2 From eb7dd7484d35e9c6f3dd32e97600040e2b70d13d Mon Sep 17 00:00:00 2001 From: onevcat Date: Sun, 2 Aug 2026 01:59:04 +0900 Subject: [PATCH 19/31] Document agent screen scan memoization --- .../000-plan.md | 17 +++++-- .../003-agent-screen-scan-memoization.md | 50 +++++++++++++++++++ docs-ai/README.md | 2 +- 3 files changed, 63 insertions(+), 6 deletions(-) create mode 100644 docs-ai/056-performance-optimization-2026-08/003-agent-screen-scan-memoization.md diff --git a/docs-ai/056-performance-optimization-2026-08/000-plan.md b/docs-ai/056-performance-optimization-2026-08/000-plan.md index 719eba90..f7b25f77 100644 --- a/docs-ai/056-performance-optimization-2026-08/000-plan.md +++ b/docs-ai/056-performance-optimization-2026-08/000-plan.md @@ -4,8 +4,8 @@ | --- | --- | | **Status** | Implemented | | **Anchor date** | 2026-08-01 | -| **Primary PRs** | #644, #645, #652, #653; review queue #646–#650 | -| **Related** | [032-performance-hardening](../032-performance-hardening/000-plan.md), [037-line-diff-tracking](../037-line-diff-tracking/000-plan.md), `docs/components/diff-view.md` | +| **Primary PRs** | #644–#646, #652, #653; review queue #647–#650 | +| **Related** | [030-agent-status-detection](../030-agent-status-detection/000-plan.md), [032-performance-hardening](../032-performance-hardening/000-plan.md), [037-line-diff-tracking](../037-line-diff-tracking/000-plan.md), `docs/components/diff-view.md` | ## Background @@ -35,6 +35,11 @@ remains wrapped for opt-in diagnostics, but a normal Debug launch now bypasses a reflection, state snapshots and equality checks, and `CustomDump` diff generation. See [056.002](002-opt-in-debug-tca-action-logging.md) for the reviewed behavior and scope. +The third application, #646, memoizes the last raw agent screen scan per terminal surface. +Polling still reads the active screen and runs process, stabilization, and session logic, but +an identical `(agent, text)` pair reuses the previous `DetectedAgent.detectState` result. See +[056.003](003-agent-screen-scan-memoization.md) for the cache boundary and remaining costs. + ## Measured baseline for #644 An independent Release-optimized benchmark on an Apple M2 Pro mirrored the production @@ -76,7 +81,7 @@ bounding this dense-match case. - No user-facing setting for byte budgets or cache policy in this wave. - No replacement of `git diff HEAD --shortstat` or the FSEvents scheduling pipeline. -- No combined implementation of #646–#650; each remains an independent review and merge +- No combined implementation of #647–#650; each remains an independent review and merge decision. - No author-reported sampling number is treated as independently verified without a same-path reproduction. @@ -122,14 +127,14 @@ input while preserving exact counts. ## Performance PR review queue -The descriptions and heads below were confirmed on 2026-08-02. Claims for #646–#650 remain +The descriptions and heads below were confirmed on 2026-08-02. Claims for #647–#650 remain pending independent code-path and test review. | PR | Confirmed scope | Review focus / placeholder | Head | | --- | --- | --- | --- | | #644 | Replace `Data.Iterator` line scans and avoid repeated large untracked-file work | Integrated through #652 with exact cached counts, a refresh-wide budget, and explicit incomplete state | `978b7b59` | | #645 | Gate Debug TCA action reflection/state-diff logging behind `PROWL_LOG_TCA_ACTIONS` | Reviewed and integrated through #653: default Debug launches bypass the expensive diagnostics; the opt-in path remains available and Release behavior is unchanged | `616bbf4b` | -| #646 | Memoize per-surface agent screen parsing when agent and visible text are unchanged | Verify cache invalidation, stabilization timing, observation isolation, and surface teardown | `b2ac2936` | +| #646 | Memoize per-surface agent screen parsing when agent and visible text are unchanged | Merged: exact agent/text cache identity preserves raw-state semantics; stabilization still runs per tick; cache lifetime follows detection/surface cleanup | `b2ac2936` | | #647 | Deduplicate raw-state-only agent emissions and narrow sidebar invalidation | Decide whether stale CLI `raw_state` is acceptable; trace UI/CLI ownership before merge | `e77ba660` | | #648 | Replace per-agent worktree scans/path resolution with a cached directory index | Verify deepest-match and symlink semantics, cache invalidation, render-path purity, and current CI | `08773383` | | #649 | Coalesce animated terminal-title writes and remove quadratic tab lookup | Verify final-title delivery, close/prune lifecycle, custom/locked titles, and clock boundaries | `b77888f3` | @@ -166,3 +171,5 @@ pending independent code-path and test review. - Updated 2026-08-02: Reviewed opt-in Debug TCA action logging from #645 and moved integration to fork-owned PR #653 — see [002-opt-in-debug-tca-action-logging.md](002-opt-in-debug-tca-action-logging.md). +- Updated 2026-08-02: Merged per-surface agent screen-scan memoization from #646 — see + [003-agent-screen-scan-memoization.md](003-agent-screen-scan-memoization.md). diff --git a/docs-ai/056-performance-optimization-2026-08/003-agent-screen-scan-memoization.md b/docs-ai/056-performance-optimization-2026-08/003-agent-screen-scan-memoization.md new file mode 100644 index 00000000..30d0b7fa --- /dev/null +++ b/docs-ai/056-performance-optimization-2026-08/003-agent-screen-scan-memoization.md @@ -0,0 +1,50 @@ +# 056.003 — Agent Screen-Scan Memoization + +## Context + +An active terminal surface with a detected agent is polled every 300 ms. Before #646, each +poll read the active terminal text and reran `DetectedAgent.detectState(in:)`, even when both +the detected agent and rendered text were unchanged. The detector narrows the input to recent +lines, then performs agent-specific splitting, normalization, and heuristic matching on the +main actor. + +The author reported that a sampled idle workload with 24 terminal surfaces attributed about +19% of main-thread time to the wider detection path, with `detectClaude` alone accounting for +roughly 10%. Those percentages are author measurements; this review verified the code path +and equivalence boundary, not the original capture. + +## Change + +- `WorktreeTerminalState.AgentScreenScan` records one `(agent, text, raw)` tuple, and + `lastAgentScreenScanBySurface` retains the latest tuple independently for each surface. + The dictionary is `@ObservationIgnored` because it is an implementation cache and does not + drive UI state. +- `detectAgentState(for:tabId:)` still probes the foreground process and calls + `readActiveText()` on every tick. If both the detected `agent` and the full active-screen + `text` match the cached tuple, it reuses the previous `AgentRawState`; otherwise it reruns + `DetectedAgent.detectState(in:)` and replaces the tuple. +- Only the pure raw screen classification is memoized. Presence handling, time-based state + stabilization, seen/unseen transitions, session resolution, diagnostics, and Active Agents + emission continue through the existing path on every applicable tick. In particular, the + working-to-idle hold can still expire while terminal text remains unchanged. +- Cache entries are removed when a cold detection task finishes, when an individual surface + is cleaned up, and during global agent-detection cleanup. +- `supacodeTests/AgentScreenScanCacheTests.swift` covers a cold scan, an exact cache hit, and + invalidation when either the text or detected agent changes. + +## Refs + +- PR #646 +- Implementation commit `b2ac2936` +- Merge commit `5a2d877f` + +## Current state + +The optimization preserves the existing classification contract because the cached raw value +is keyed by every input to the pure detector. It trades one retained active-screen string per +detected surface for avoiding repeated parsing and normalization of identical text. + +The cache does not eliminate the complete polling cost: Ghostty text extraction, exact string +comparison, process detection, stabilization, and session matching still run as before. A +future optimization would need a reliable Ghostty screen-generation signal to bypass text +materialization itself; #646 intentionally does not introduce that broader integration. diff --git a/docs-ai/README.md b/docs-ai/README.md index 3d44b406..8cd5651f 100644 --- a/docs-ai/README.md +++ b/docs-ai/README.md @@ -106,4 +106,4 @@ agent-facing manual for that). | 052 | [sidebar-context-menus](052-sidebar-context-menus/000-plan.md) | 2026-07-27 | Sidebar context menu overhaul: worktree terminal actions, header/workspace path actions, PR click-through | | 053 | [agent-profiles](053-agent-profiles/000-plan.md) | 2026-07-29 | Agent Profile V1: preset-first Claude Code/Codex launch profiles, opt-in account isolation, path routing, and future handoff seam | | 054 | [native-settings-navigation](054-native-settings-navigation/000-plan.md) | 2026-07-30 | SwiftUI-owned singleton Settings window, native sidebar/detail navigation, and Agent Profile drill-ins | -| 056 | [performance-optimization-2026-08](056-performance-optimization-2026-08/000-plan.md) | 2026-08-01 | August performance review series: exact cached line counts and opt-in Debug TCA action logging | +| 056 | [performance-optimization-2026-08](056-performance-optimization-2026-08/000-plan.md) | 2026-08-01 | August performance review series: exact cached line counts, opt-in Debug TCA logging, and agent screen-scan memoization | -- 2.51.2 From d209df960b5a8941a4f358fb6d514428404ebfe9 Mon Sep 17 00:00:00 2001 From: onevcat Date: Sun, 2 Aug 2026 02:21:07 +0900 Subject: [PATCH 20/31] Preserve live CLI raw agent state --- supacode/App/supacodeApp.swift | 11 +++++++++-- supacode/CLIService/AgentsCommandHandler.swift | 6 +++++- .../Features/Repositories/Views/SidebarListView.swift | 1 - supacodeTests/AgentEntryEmissionDedupTests.swift | 5 ++++- supacodeTests/CLIAgentsCommandHandlerTests.swift | 8 ++++++-- 5 files changed, 24 insertions(+), 7 deletions(-) diff --git a/supacode/App/supacodeApp.swift b/supacode/App/supacodeApp.swift index 5b47acd0..b3544456 100644 --- a/supacode/App/supacodeApp.swift +++ b/supacode/App/supacodeApp.swift @@ -541,12 +541,19 @@ struct SupacodeApp: App { ) } let agentsHandler = AgentsCommandHandler { - AgentsRuntimeSnapshot( + var rawStatesBySurfaceID: [UUID: AgentRawState] = [:] + for terminalState in terminalManager.activeWorktreeStates { + for (surfaceID, agentState) in terminalState.surfaceAgentStates { + rawStatesBySurfaceID[surfaceID] = agentState.fallbackState + } + } + return AgentsRuntimeSnapshot( repositoriesState: appStore.state.repositories, listSnapshot: ListRuntimeSnapshotBuilder.makeSnapshot( repositoriesState: appStore.state.repositories, terminalManager: terminalManager - ) + ), + rawStatesBySurfaceID: rawStatesBySurfaceID ) } let sendHandler = SendCommandHandler( diff --git a/supacode/CLIService/AgentsCommandHandler.swift b/supacode/CLIService/AgentsCommandHandler.swift index 4290233c..0ce2e2d3 100644 --- a/supacode/CLIService/AgentsCommandHandler.swift +++ b/supacode/CLIService/AgentsCommandHandler.swift @@ -3,6 +3,10 @@ import Foundation struct AgentsRuntimeSnapshot { let repositoriesState: RepositoriesFeature.State let listSnapshot: ListRuntimeSnapshot + /// Live detector output keyed by pane. Reducer entries intentionally skip + /// raw-state-only changes to avoid invalidating the sidebar, so CLI snapshots + /// must source this field from terminal state instead. + let rawStatesBySurfaceID: [UUID: AgentRawState] } final class AgentsCommandHandler: CommandHandler { @@ -83,7 +87,7 @@ final class AgentsCommandHandler: CommandHandler { type: entry.agent.rawValue, name: entry.displayName, status: AgentsCommandStatus(rawValue: entry.displayState.rawValue) ?? .idle, - rawState: entry.rawState.rawValue, + rawState: (snapshot.rawStatesBySurfaceID[entry.surfaceID] ?? entry.rawState).rawValue, lastChangedAt: dateFormatter.string(from: entry.lastChangedAt), project: AgentsCommandProject( name: display.repositoryName, diff --git a/supacode/Features/Repositories/Views/SidebarListView.swift b/supacode/Features/Repositories/Views/SidebarListView.swift index c68d2bc7..d5aa04bd 100644 --- a/supacode/Features/Repositories/Views/SidebarListView.swift +++ b/supacode/Features/Repositories/Views/SidebarListView.swift @@ -62,7 +62,6 @@ struct SidebarListView: View { @State private var isAddChoicePresented = false @Namespace private var topSegmentNamespace @Environment(\.resolvedKeybindings) private var resolvedKeybindings - @Shared(.repositoryAppearances) private var repositoryAppearances var body: some View { let state = store.state diff --git a/supacodeTests/AgentEntryEmissionDedupTests.swift b/supacodeTests/AgentEntryEmissionDedupTests.swift index 86c951a8..d974a4b0 100644 --- a/supacodeTests/AgentEntryEmissionDedupTests.swift +++ b/supacodeTests/AgentEntryEmissionDedupTests.swift @@ -90,6 +90,7 @@ struct AgentEntryEmissionDedupTests { ("paneIndex", Self.entry(paneIndex: 2)), ("iconLookupToken", Self.entry(iconLookupToken: "codex")), ("agent", Self.entry(agent: .codex)), + ("launchProfileName", Self.entry(launchProfileName: "Review Profile")), // Carries the resume target `prowl agents` reports; a suppressed change // would keep pointing at the previous session's transcript. ("session", Self.entry(session: Self.otherSession)), @@ -141,6 +142,7 @@ struct AgentEntryEmissionDedupTests { paneIndex: Int = 1, iconLookupToken: String = "claude", agent: DetectedAgent = .claude, + launchProfileName: String? = nil, session: AgentSession? = baseSession, rawState: AgentRawState = .working, displayState: AgentDisplayState = .working, @@ -160,7 +162,8 @@ struct AgentEntryEmissionDedupTests { session: session, rawState: rawState, displayState: displayState, - lastChangedAt: lastChangedAt + lastChangedAt: lastChangedAt, + launchProfileName: launchProfileName ) } diff --git a/supacodeTests/CLIAgentsCommandHandlerTests.swift b/supacodeTests/CLIAgentsCommandHandlerTests.swift index d3cb80a5..1483a984 100644 --- a/supacodeTests/CLIAgentsCommandHandlerTests.swift +++ b/supacodeTests/CLIAgentsCommandHandlerTests.swift @@ -29,7 +29,10 @@ struct CLIAgentsCommandHandlerTests { #expect(agent.type == "pi") #expect(agent.name == "omp") #expect(agent.status == .blocked) - #expect(agent.rawState == "blocked") + // The reducer entry may intentionally lag raw-state-only terminal polls so + // the sidebar does not re-render. CLI snapshots must use the live terminal + // raw state instead of that UI-deduplicated value. + #expect(agent.rawState == "working") #expect(agent.lastChangedAt == "2026-09-21T14:00:00Z") #expect(agent.project.name == "Prowl") #expect(agent.project.branch == "feature/agents") @@ -179,7 +182,8 @@ struct CLIAgentsCommandHandlerTests { otherPaneID: otherPaneID, tabID: tabID, tabWorktree: tabWorktree - ) + ), + rawStatesBySurfaceID: [tabPaneID: .working] ) ) } -- 2.51.2 From 3eb24526b8fb19228d007f60130395f31922fa10 Mon Sep 17 00:00:00 2001 From: onevcat Date: Sun, 2 Aug 2026 02:24:34 +0900 Subject: [PATCH 21/31] Document agent entry emission review --- .../000-plan.md | 7 ++- .../004-agent-entry-emission-dedup.md | 58 +++++++++++++++++++ 2 files changed, 63 insertions(+), 2 deletions(-) create mode 100644 docs-ai/056-performance-optimization-2026-08/004-agent-entry-emission-dedup.md diff --git a/docs-ai/056-performance-optimization-2026-08/000-plan.md b/docs-ai/056-performance-optimization-2026-08/000-plan.md index f7b25f77..4ce066f5 100644 --- a/docs-ai/056-performance-optimization-2026-08/000-plan.md +++ b/docs-ai/056-performance-optimization-2026-08/000-plan.md @@ -4,7 +4,7 @@ | --- | --- | | **Status** | Implemented | | **Anchor date** | 2026-08-01 | -| **Primary PRs** | #644–#646, #652, #653; review queue #647–#650 | +| **Primary PRs** | #644–#647, #652, #653; review queue #648–#650 | | **Related** | [030-agent-status-detection](../030-agent-status-detection/000-plan.md), [032-performance-hardening](../032-performance-hardening/000-plan.md), [037-line-diff-tracking](../037-line-diff-tracking/000-plan.md), `docs/components/diff-view.md` | ## Background @@ -135,7 +135,7 @@ pending independent code-path and test review. | #644 | Replace `Data.Iterator` line scans and avoid repeated large untracked-file work | Integrated through #652 with exact cached counts, a refresh-wide budget, and explicit incomplete state | `978b7b59` | | #645 | Gate Debug TCA action reflection/state-diff logging behind `PROWL_LOG_TCA_ACTIONS` | Reviewed and integrated through #653: default Debug launches bypass the expensive diagnostics; the opt-in path remains available and Release behavior is unchanged | `616bbf4b` | | #646 | Memoize per-surface agent screen parsing when agent and visible text are unchanged | Merged: exact agent/text cache identity preserves raw-state semantics; stabilization still runs per tick; cache lifetime follows detection/surface cleanup | `b2ac2936` | -| #647 | Deduplicate raw-state-only agent emissions and narrow sidebar invalidation | Decide whether stale CLI `raw_state` is acceptable; trace UI/CLI ownership before merge | `e77ba660` | +| #647 | Deduplicate raw-state-only agent emissions and narrow sidebar invalidation | Reviewed with follow-up: UI emission ignores raw-only churn while `prowl agents` reads live terminal raw state; full-field guard includes Profile attribution | `e77ba660` | | #648 | Replace per-agent worktree scans/path resolution with a cached directory index | Verify deepest-match and symlink semantics, cache invalidation, render-path purity, and current CI | `08773383` | | #649 | Coalesce animated terminal-title writes and remove quadratic tab lookup | Verify final-title delivery, close/prune lifecycle, custom/locked titles, and clock boundaries | `b77888f3` | | #650 | Cache parsed transcript tails and fast-path fingerprint normalization | Verify append/mtime invalidation, cache pruning, Unicode equivalence, collision behavior, and current CI | `c97cbb4` | @@ -173,3 +173,6 @@ pending independent code-path and test review. [002-opt-in-debug-tca-action-logging.md](002-opt-in-debug-tca-action-logging.md). - Updated 2026-08-02: Merged per-surface agent screen-scan memoization from #646 — see [003-agent-screen-scan-memoization.md](003-agent-screen-scan-memoization.md). +- Updated 2026-08-02: Reviewed agent-entry emission deduplication from #647 and preserved the + live CLI raw-state contract in the fork follow-up — see + [004-agent-entry-emission-dedup.md](004-agent-entry-emission-dedup.md). diff --git a/docs-ai/056-performance-optimization-2026-08/004-agent-entry-emission-dedup.md b/docs-ai/056-performance-optimization-2026-08/004-agent-entry-emission-dedup.md new file mode 100644 index 00000000..b75f2b8b --- /dev/null +++ b/docs-ai/056-performance-optimization-2026-08/004-agent-entry-emission-dedup.md @@ -0,0 +1,58 @@ +# 056.004 — Agent Entry Emission Deduplication + +## Context + +Agent detection polls active surfaces as frequently as every 300 ms. The raw detector state +can oscillate with an agent's animated terminal output even when the stabilized display state, +session, working directory, and every other consumer-visible field remain unchanged. Before +#647, each raw-only change emitted a new `ActiveAgentEntry`, traversed the terminal event stream, +updated `ActiveAgentsFeature.State.entries`, and invalidated the sidebar view that read the +entries while constructing the repository list. + +The author measured nine `SidebarListView.body` evaluations and 99 repository-section +evaluations over an eight-second working-agent capture. Those counts are author measurements; +the review verified the emission and observation path, not the original instrumented capture. + +## Change + +- #647 adds `ActiveAgentEntry.equalsIgnoringRawState(_:)` and uses it in + `WorktreeTerminalState.emitAgentEntry`. A raw-state-only change remains in the terminal-owned + `surfaceAgentStates` snapshot but no longer emits through TCA. +- `SidebarActiveAgentsOverlay` owns the `activeAgents.entries` read and row-display computation. + Entry changes therefore invalidate the overlay rather than the parent + `SidebarListView.body` that constructs every repository section. +- The original PR would also have made `prowl agents` report the raw state from the most recent + visible entry change. The fork follow-up keeps the existing point-in-time CLI contract: + `AgentsRuntimeSnapshot` captures live `PaneAgentState.fallbackState` values from + `WorktreeTerminalManager`, and `AgentsCommandHandler` uses that value when available while + retaining the reducer entry as a defensive fallback. +- The emission-equivalence regression test changes every stored `ActiveAgentEntry` field one at + a time. The follow-up adds the previously omitted `launchProfileName`, which affects the agent + display name and must force a new entry. + +## Refs + +- Original PR #647 +- Original implementation commits `7903375c` and `e77ba660` +- Fork follow-up commit `d209df96` + +## Current state + +Raw detector flicker no longer crosses the terminal-to-TCA event boundary or invalidates the +sidebar. Stabilized status, session, title, working-directory, Profile attribution, and other +entry changes still emit normally. CLI requests read the latest terminal-owned raw state on the +main actor, so the optimization does not turn `raw_state` into a delayed value. + +The follow-up deliberately does not add another raw-state stream or dependency client. CLI +snapshot construction already receives `WorktreeTerminalManager`, and the live state is copied +only when a CLI request arrives. + +## Verification + +- The original focused `AgentEntryEmissionDedupTests` and `CLIAgentsCommandHandlerTests` passed + seven tests before the follow-up. +- A CLI test was changed first to require a live raw state different from the reducer entry; it + failed to compile until the live-state snapshot input existed. +- The focused suites then passed seven tests, including raw-only deduplication, visible-state + emission, removal/re-attachment, full-field equality protection, and live CLI raw-state + precedence. -- 2.51.2 From 65bff41a1c8e40e1fad320506a5003f0aaeec3f0 Mon Sep 17 00:00:00 2001 From: onevcat Date: Sun, 2 Aug 2026 02:25:39 +0900 Subject: [PATCH 22/31] Link sidebar churn follow-up PR --- docs-ai/056-performance-optimization-2026-08/000-plan.md | 4 ++-- .../004-agent-entry-emission-dedup.md | 1 + 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/docs-ai/056-performance-optimization-2026-08/000-plan.md b/docs-ai/056-performance-optimization-2026-08/000-plan.md index 4ce066f5..1d87602c 100644 --- a/docs-ai/056-performance-optimization-2026-08/000-plan.md +++ b/docs-ai/056-performance-optimization-2026-08/000-plan.md @@ -4,7 +4,7 @@ | --- | --- | | **Status** | Implemented | | **Anchor date** | 2026-08-01 | -| **Primary PRs** | #644–#647, #652, #653; review queue #648–#650 | +| **Primary PRs** | #644–#647, #652–#654; review queue #648–#650 | | **Related** | [030-agent-status-detection](../030-agent-status-detection/000-plan.md), [032-performance-hardening](../032-performance-hardening/000-plan.md), [037-line-diff-tracking](../037-line-diff-tracking/000-plan.md), `docs/components/diff-view.md` | ## Background @@ -135,7 +135,7 @@ pending independent code-path and test review. | #644 | Replace `Data.Iterator` line scans and avoid repeated large untracked-file work | Integrated through #652 with exact cached counts, a refresh-wide budget, and explicit incomplete state | `978b7b59` | | #645 | Gate Debug TCA action reflection/state-diff logging behind `PROWL_LOG_TCA_ACTIONS` | Reviewed and integrated through #653: default Debug launches bypass the expensive diagnostics; the opt-in path remains available and Release behavior is unchanged | `616bbf4b` | | #646 | Memoize per-surface agent screen parsing when agent and visible text are unchanged | Merged: exact agent/text cache identity preserves raw-state semantics; stabilization still runs per tick; cache lifetime follows detection/surface cleanup | `b2ac2936` | -| #647 | Deduplicate raw-state-only agent emissions and narrow sidebar invalidation | Reviewed with follow-up: UI emission ignores raw-only churn while `prowl agents` reads live terminal raw state; full-field guard includes Profile attribution | `e77ba660` | +| #647 | Deduplicate raw-state-only agent emissions and narrow sidebar invalidation | Reviewed and integrated through #654: UI emission ignores raw-only churn while `prowl agents` reads live terminal raw state; full-field guard includes Profile attribution | `e77ba660` | | #648 | Replace per-agent worktree scans/path resolution with a cached directory index | Verify deepest-match and symlink semantics, cache invalidation, render-path purity, and current CI | `08773383` | | #649 | Coalesce animated terminal-title writes and remove quadratic tab lookup | Verify final-title delivery, close/prune lifecycle, custom/locked titles, and clock boundaries | `b77888f3` | | #650 | Cache parsed transcript tails and fast-path fingerprint normalization | Verify append/mtime invalidation, cache pruning, Unicode equivalence, collision behavior, and current CI | `c97cbb4` | diff --git a/docs-ai/056-performance-optimization-2026-08/004-agent-entry-emission-dedup.md b/docs-ai/056-performance-optimization-2026-08/004-agent-entry-emission-dedup.md index b75f2b8b..c5decc9d 100644 --- a/docs-ai/056-performance-optimization-2026-08/004-agent-entry-emission-dedup.md +++ b/docs-ai/056-performance-optimization-2026-08/004-agent-entry-emission-dedup.md @@ -33,6 +33,7 @@ the review verified the emission and observation path, not the original instrume ## Refs - Original PR #647 +- Fork integration PR #654 - Original implementation commits `7903375c` and `e77ba660` - Fork follow-up commit `d209df96` -- 2.51.2 From b39b7ff4ec11f422ebf6541782b908342197abbd Mon Sep 17 00:00:00 2001 From: onevcat Date: Sun, 2 Aug 2026 02:34:02 +0900 Subject: [PATCH 23/31] Revalidate cached worktree symlink targets --- .../003-sidebar-agent-row-resolution.md | 22 +++++--- supacode/Domain/WorktreeDirectoryIndex.swift | 51 +++++++++++++++---- .../WorktreeDirectoryIndexTests.swift | 32 ++++++++++++ 3 files changed, 88 insertions(+), 17 deletions(-) diff --git a/docs-ai/032-performance-hardening/003-sidebar-agent-row-resolution.md b/docs-ai/032-performance-hardening/003-sidebar-agent-row-resolution.md index bc9cad5b..232056a1 100644 --- a/docs-ai/032-performance-hardening/003-sidebar-agent-row-resolution.md +++ b/docs-ai/032-performance-hardening/003-sidebar-agent-row-resolution.md @@ -48,14 +48,18 @@ permanently lengthened the inner loop. directory still wins. Keys compare whole components rather than raw string prefixes, which keeps `/tmp/repo` from matching a sibling `/tmp/repo2`. - `WorktreeDirectoryIndexCache` memoizes the index against the `(id, directory)` pairs it was - built from. Render passes that change only branch names, colors, or icons reuse it. + built from. Render passes that change only branch names, colors, or icons reuse it. Because + canonical paths also depend on filesystem state, an unchanged repository set revalidates its + normalized directories at most once per second and rebuilds if a symlink target changed. `SidebarListView.activeAgentRowDisplays` now resolves the index once for the whole batch. `resolveWorktreeID` is kept as the public entry point and delegates to the cache. Per sidebar render the cost drops from `agents × worktrees × 3` calls to `normalizeURL` — two -filesystem round-trips each — to zero filesystem calls while the repository set is unchanged, -and `worktrees + 1` on the render after it changes. +filesystem round-trips each — to one queried-directory normalization per agent. Candidate +directories are normalized when the repository set changes and, while it stays stable, no more +than once per second for canonical-path validation. This bounds external symlink-target staleness +without restoring filesystem work to every render pass. Resolution semantics are unchanged, including the pre-existing asymmetry where a non-existent path skips symlink resolution while an existing one does not. The 13 sidebar @@ -63,15 +67,15 @@ tests in `supacodeTests/RepositorySectionViewTests.swift` pass unmodified. ## Refs -Commit `ddb203e6` on branch `perf/sidebar-worktree-resolution`. PR pending — the branch was -held for local verification before opening one. +PR #648, implementation commit `246373b6`. ## Current state -`supacode/Domain/WorktreeDirectoryIndex.swift` owns the resolution. Eight tests in +`supacode/Domain/WorktreeDirectoryIndex.swift` owns the resolution. Ten tests in `supacodeTests/WorktreeDirectoryIndexTests.swift` cover nested resolution, deepest-match preference, the sibling-prefix trap, plain-folder repositories, unmatched paths, the empty -index, `..` normalization, and both cache paths. +index, `..` normalization, repository-set invalidation, branch-only reuse, and live symlink +retargeting after the bounded revalidation interval. `make build-app` reported 0 errors and 0 warnings, `make test` reported 1943 passing tests and 0 failures, and `make check` (swift-format strict lint plus SwiftLint) was clean. @@ -90,7 +94,9 @@ Process CPU fell from 94–134% to 29–70%. The workloads were not identical sessions before versus 23 surfaces and 8 sessions after — so the percentages are indicative rather than a controlled comparison; the disappearance of `resolveWorktreeID` from the profile is not. The only residual frames are 24 samples (0.3%) in `WorktreeDirectoryIndex.worktreeID`, -which is the by-design single normalization of the queried directory per lookup. +which is the by-design single normalization of the queried directory per lookup. This profile +predates the once-per-second canonical revalidation added during review; that bounded validation +cost has not been independently sampled. ## Recurrence note diff --git a/supacode/Domain/WorktreeDirectoryIndex.swift b/supacode/Domain/WorktreeDirectoryIndex.swift index 2732e2ff..d9aeaa37 100644 --- a/supacode/Domain/WorktreeDirectoryIndex.swift +++ b/supacode/Domain/WorktreeDirectoryIndex.swift @@ -28,8 +28,18 @@ nonisolated struct WorktreeDirectoryIndex: Equatable { } } + fileprivate init(normalizedDirectories: [(id: Worktree.ID, directory: URL)]) { + for entry in normalizedDirectories { + insertNormalized(id: entry.id, directory: entry.directory) + } + } + private mutating func insert(id: Worktree.ID, directory: URL) { - let components = PathPolicy.normalizeURL(directory).pathComponents + insertNormalized(id: id, directory: PathPolicy.normalizeURL(directory)) + } + + private mutating func insertNormalized(id: Worktree.ID, directory: URL) { + let components = directory.pathComponents let key = Self.key(for: components) // The first entry registered for a directory wins. This preserves the previous behavior when a // plain-folder repository root and its main worktree resolve to the same path. @@ -64,25 +74,46 @@ nonisolated struct WorktreeDirectoryIndex: Equatable { /// Memoizes the index across SwiftUI render passes. /// /// `SidebarListView.body` re-runs on every agent output tick, and rebuilding the index each time -/// would put one filesystem round-trip per worktree back on the main thread. The index is a pure -/// function of the repository set, so a cached copy stays valid until that set changes. +/// would put one filesystem round-trip per worktree back on the main thread. Repository changes +/// rebuild immediately; otherwise canonical paths are revalidated at a bounded cadence so a live +/// symlink retarget cannot leave the index stale until restart. @MainActor enum WorktreeDirectoryIndexCache { - private static var cachedSignature: [Entry] = [] + private static let canonicalRevalidationInterval: Duration = .seconds(1) + private static var cachedSignature: [Entry]? + private static var cachedCanonicalSignature: [Entry] = [] private static var cachedIndex = WorktreeDirectoryIndex() + private static var nextCanonicalRevalidation: ContinuousClock.Instant? private struct Entry: Equatable { let id: Worktree.ID let directory: URL } - static func index(for repositories: IdentifiedArrayOf) -> WorktreeDirectoryIndex { + static func index( + for repositories: IdentifiedArrayOf, + now: ContinuousClock.Instant = ContinuousClock.now + ) -> WorktreeDirectoryIndex { let signature = signature(for: repositories) - guard signature == cachedSignature else { - cachedSignature = signature - cachedIndex = WorktreeDirectoryIndex(repositories: repositories) + let repositorySetChanged = signature != cachedSignature + let canonicalRevalidationIsDue = nextCanonicalRevalidation.map { now >= $0 } ?? true + guard repositorySetChanged || canonicalRevalidationIsDue else { return cachedIndex } + + let canonicalSignature = signature.map { entry in + Entry(id: entry.id, directory: PathPolicy.normalizeURL(entry.directory)) + } + cachedSignature = signature + nextCanonicalRevalidation = now.advanced(by: canonicalRevalidationInterval) + guard canonicalSignature != cachedCanonicalSignature else { + return cachedIndex + } + + cachedCanonicalSignature = canonicalSignature + cachedIndex = WorktreeDirectoryIndex( + normalizedDirectories: canonicalSignature.map { (id: $0.id, directory: $0.directory) } + ) return cachedIndex } @@ -105,7 +136,9 @@ enum WorktreeDirectoryIndexCache { /// Drops the memo so a test starts from a known state. static func reset() { - cachedSignature = [] + cachedSignature = nil + cachedCanonicalSignature = [] cachedIndex = WorktreeDirectoryIndex() + nextCanonicalRevalidation = nil } } diff --git a/supacodeTests/WorktreeDirectoryIndexTests.swift b/supacodeTests/WorktreeDirectoryIndexTests.swift index 7abdef32..0dce6308 100644 --- a/supacodeTests/WorktreeDirectoryIndexTests.swift +++ b/supacodeTests/WorktreeDirectoryIndexTests.swift @@ -116,6 +116,38 @@ struct WorktreeDirectoryIndexTests { #expect(first == second) } + @Test func cacheRevalidatesCanonicalPathsAfterInterval() throws { + WorktreeDirectoryIndexCache.reset() + let tempRoot = FileManager.default.temporaryDirectory + .appending(path: UUID().uuidString, directoryHint: .isDirectory) + let firstTarget = tempRoot.appending(path: "first", directoryHint: .isDirectory) + let secondTarget = tempRoot.appending(path: "second", directoryHint: .isDirectory) + let symlink = tempRoot.appending(path: "current", directoryHint: .isDirectory) + defer { try? FileManager.default.removeItem(at: tempRoot) } + try FileManager.default.createDirectory(at: firstTarget, withIntermediateDirectories: true) + try FileManager.default.createDirectory(at: secondTarget, withIntermediateDirectories: true) + try FileManager.default.createSymbolicLink(at: symlink, withDestinationURL: firstTarget) + + let repository = makeRepository( + id: symlink.path(percentEncoded: false), + kind: .plain, + worktrees: [] + ) + let start = ContinuousClock.now + let first = WorktreeDirectoryIndexCache.index(for: [repository], now: start) + #expect(first.worktreeID(forWorkingDirectory: firstTarget) == repository.id) + + try FileManager.default.removeItem(at: symlink) + try FileManager.default.createSymbolicLink(at: symlink, withDestinationURL: secondTarget) + + let revalidated = WorktreeDirectoryIndexCache.index( + for: [repository], + now: start.advanced(by: .seconds(1)) + ) + #expect(revalidated.worktreeID(forWorkingDirectory: secondTarget) == repository.id) + #expect(revalidated.worktreeID(forWorkingDirectory: firstTarget) == nil) + } + // MARK: - Helpers private func makeWorktree(repoRoot: String, path: String, branch: String) -> Worktree { -- 2.51.2 From 11f1eb4e908a4f0a4595a1d2a340958ccb333e4f Mon Sep 17 00:00:00 2001 From: onevcat Date: Sun, 2 Aug 2026 02:36:54 +0900 Subject: [PATCH 24/31] Document worktree directory index review --- .../000-plan.md | 7 ++- .../005-worktree-directory-index.md | 63 +++++++++++++++++++ 2 files changed, 68 insertions(+), 2 deletions(-) create mode 100644 docs-ai/056-performance-optimization-2026-08/005-worktree-directory-index.md diff --git a/docs-ai/056-performance-optimization-2026-08/000-plan.md b/docs-ai/056-performance-optimization-2026-08/000-plan.md index f7b25f77..6863e5d8 100644 --- a/docs-ai/056-performance-optimization-2026-08/000-plan.md +++ b/docs-ai/056-performance-optimization-2026-08/000-plan.md @@ -4,7 +4,7 @@ | --- | --- | | **Status** | Implemented | | **Anchor date** | 2026-08-01 | -| **Primary PRs** | #644–#646, #652, #653; review queue #647–#650 | +| **Primary PRs** | #644–#646, #648, #652, #653; review queue #647, #649, #650 | | **Related** | [030-agent-status-detection](../030-agent-status-detection/000-plan.md), [032-performance-hardening](../032-performance-hardening/000-plan.md), [037-line-diff-tracking](../037-line-diff-tracking/000-plan.md), `docs/components/diff-view.md` | ## Background @@ -136,7 +136,7 @@ pending independent code-path and test review. | #645 | Gate Debug TCA action reflection/state-diff logging behind `PROWL_LOG_TCA_ACTIONS` | Reviewed and integrated through #653: default Debug launches bypass the expensive diagnostics; the opt-in path remains available and Release behavior is unchanged | `616bbf4b` | | #646 | Memoize per-surface agent screen parsing when agent and visible text are unchanged | Merged: exact agent/text cache identity preserves raw-state semantics; stabilization still runs per tick; cache lifetime follows detection/surface cleanup | `b2ac2936` | | #647 | Deduplicate raw-state-only agent emissions and narrow sidebar invalidation | Decide whether stale CLI `raw_state` is acceptable; trace UI/CLI ownership before merge | `e77ba660` | -| #648 | Replace per-agent worktree scans/path resolution with a cached directory index | Verify deepest-match and symlink semantics, cache invalidation, render-path purity, and current CI | `08773383` | +| #648 | Replace per-agent worktree scans/path resolution with a cached directory index | Reviewed with follow-up: deepest-match semantics hold; canonical paths revalidate at a bounded cadence so live symlink retargets cannot stale the cache indefinitely | `08773383` | | #649 | Coalesce animated terminal-title writes and remove quadratic tab lookup | Verify final-title delivery, close/prune lifecycle, custom/locked titles, and clock boundaries | `b77888f3` | | #650 | Cache parsed transcript tails and fast-path fingerprint normalization | Verify append/mtime invalidation, cache pruning, Unicode equivalence, collision behavior, and current CI | `c97cbb4` | @@ -173,3 +173,6 @@ pending independent code-path and test review. [002-opt-in-debug-tca-action-logging.md](002-opt-in-debug-tca-action-logging.md). - Updated 2026-08-02: Merged per-surface agent screen-scan memoization from #646 — see [003-agent-screen-scan-memoization.md](003-agent-screen-scan-memoization.md). +- Updated 2026-08-02: Reviewed the cached worktree directory index from #648 and added + bounded canonical-path revalidation — see + [005-worktree-directory-index.md](005-worktree-directory-index.md). diff --git a/docs-ai/056-performance-optimization-2026-08/005-worktree-directory-index.md b/docs-ai/056-performance-optimization-2026-08/005-worktree-directory-index.md new file mode 100644 index 00000000..20578aaa --- /dev/null +++ b/docs-ai/056-performance-optimization-2026-08/005-worktree-directory-index.md @@ -0,0 +1,63 @@ +# 056.005 — Cached Worktree Directory Index + +## Context + +Active Agents rows derive their displayed repository and branch from the agent pane's current +working directory. Before #648, every row scanned every repository worktree and repeatedly called +`PathPolicy.normalizeURL`, including filesystem existence checks and symlink resolution, from the +sidebar render path. + +The author sampled a working instance with six agents and 34 terminal surfaces and attributed +2,253 of 4,711 samples to `SidebarListView.resolveWorktreeID`. A subsequent build removed that +symbol from the profile and reduced `SidebarListView.body` from 51% to 2.4% of main-thread samples. +The workloads were not identical, and this review verified the structural hot path rather than +reproducing those percentages. + +## Change + +- #648 introduces `WorktreeDirectoryIndex`. Candidate repository and worktree directories are + normalized into a component-keyed dictionary; lookups normalize the queried directory once and + walk upward until the deepest indexed path matches. +- Component keys preserve the prior containment semantics without raw-prefix errors such as + matching `/tmp/repo2` to `/tmp/repo`. Registration order preserves the previous winner when two + entries normalize to the same depth and path. +- `WorktreeDirectoryIndexCache` reuses the index when repository IDs and stored directories do not + change, avoiding repeated candidate normalization on ordinary SwiftUI invalidations. +- The original cache treated canonical paths as a pure function of the stored URL. That is false + for a plain-folder root that is a symlink: retargeting it in place leaves `(id, URL)` unchanged + while `PathPolicy.normalizeURL` produces a new path. The fork follow-up immediately rebuilds for + repository-set changes and otherwise revalidates canonical candidate paths at most once per + second. A changed target rebuilds the index from the already-normalized signature. + +## Refs + +- Original PR #648 +- Original implementation commit `246373b6` +- Fork follow-up commit `b39b7ff4` +- Detailed performance record: + [032.003](../032-performance-hardening/003-sidebar-agent-row-resolution.md) + +## Current state + +Row-display batches share one cached index, replacing the former +`agents × candidate directories` scan with one queried-directory normalization per row and +dictionary probes. Candidate canonicalization occurs when repository paths change and, for +external filesystem changes, no more than once per second while renders continue. + +The one-second cadence is an explicit correctness/performance tradeoff: a live plain-folder +symlink retarget can keep the previous association until the first render at or after the next +validation boundary, but it no longer remains stale until an unrelated repository change or app +restart. The original before/after profile predates this review addition, so its bounded cost has +not been independently sampled. + +## Verification + +- The original focused worktree-index, row-display, repository-section, and CLI suites passed 12 + tests before the follow-up. +- A symlink-retarget test was added first and failed to compile until the cache accepted an + injectable `ContinuousClock.Instant`. +- `WorktreeDirectoryIndexTests` then passed 10 tests, including a real temporary symlink retarget + driven across the validation boundary without sleeping. +- The combined index, active-agent working-directory, repository-section, and CLI suites passed 30 + tests with no failures or warnings. +- `make check` passed before integrating the latest `main`. -- 2.51.2 From 93232628d7fd3d39929d9468fa729c43c10aaca9 Mon Sep 17 00:00:00 2001 From: onevcat Date: Sun, 2 Aug 2026 02:37:29 +0900 Subject: [PATCH 25/31] Link worktree index follow-up PR --- docs-ai/056-performance-optimization-2026-08/000-plan.md | 4 ++-- .../005-worktree-directory-index.md | 1 + 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/docs-ai/056-performance-optimization-2026-08/000-plan.md b/docs-ai/056-performance-optimization-2026-08/000-plan.md index 6863e5d8..0f90c502 100644 --- a/docs-ai/056-performance-optimization-2026-08/000-plan.md +++ b/docs-ai/056-performance-optimization-2026-08/000-plan.md @@ -4,7 +4,7 @@ | --- | --- | | **Status** | Implemented | | **Anchor date** | 2026-08-01 | -| **Primary PRs** | #644–#646, #648, #652, #653; review queue #647, #649, #650 | +| **Primary PRs** | #644–#646, #648, #652, #653, #655; review queue #647, #649, #650 | | **Related** | [030-agent-status-detection](../030-agent-status-detection/000-plan.md), [032-performance-hardening](../032-performance-hardening/000-plan.md), [037-line-diff-tracking](../037-line-diff-tracking/000-plan.md), `docs/components/diff-view.md` | ## Background @@ -136,7 +136,7 @@ pending independent code-path and test review. | #645 | Gate Debug TCA action reflection/state-diff logging behind `PROWL_LOG_TCA_ACTIONS` | Reviewed and integrated through #653: default Debug launches bypass the expensive diagnostics; the opt-in path remains available and Release behavior is unchanged | `616bbf4b` | | #646 | Memoize per-surface agent screen parsing when agent and visible text are unchanged | Merged: exact agent/text cache identity preserves raw-state semantics; stabilization still runs per tick; cache lifetime follows detection/surface cleanup | `b2ac2936` | | #647 | Deduplicate raw-state-only agent emissions and narrow sidebar invalidation | Decide whether stale CLI `raw_state` is acceptable; trace UI/CLI ownership before merge | `e77ba660` | -| #648 | Replace per-agent worktree scans/path resolution with a cached directory index | Reviewed with follow-up: deepest-match semantics hold; canonical paths revalidate at a bounded cadence so live symlink retargets cannot stale the cache indefinitely | `08773383` | +| #648 | Replace per-agent worktree scans/path resolution with a cached directory index | Reviewed and integrated through #655: deepest-match semantics hold; canonical paths revalidate at a bounded cadence so live symlink retargets cannot stale the cache indefinitely | `08773383` | | #649 | Coalesce animated terminal-title writes and remove quadratic tab lookup | Verify final-title delivery, close/prune lifecycle, custom/locked titles, and clock boundaries | `b77888f3` | | #650 | Cache parsed transcript tails and fast-path fingerprint normalization | Verify append/mtime invalidation, cache pruning, Unicode equivalence, collision behavior, and current CI | `c97cbb4` | diff --git a/docs-ai/056-performance-optimization-2026-08/005-worktree-directory-index.md b/docs-ai/056-performance-optimization-2026-08/005-worktree-directory-index.md index 20578aaa..a4230bee 100644 --- a/docs-ai/056-performance-optimization-2026-08/005-worktree-directory-index.md +++ b/docs-ai/056-performance-optimization-2026-08/005-worktree-directory-index.md @@ -32,6 +32,7 @@ reproducing those percentages. ## Refs - Original PR #648 +- Fork integration PR #655 - Original implementation commit `246373b6` - Fork follow-up commit `b39b7ff4` - Detailed performance record: -- 2.51.2 From 5c7e2a35c15eb5eecf9aac325afade75bbc3510c Mon Sep 17 00:00:00 2001 From: onevcat Date: Sun, 2 Aug 2026 03:03:45 +0900 Subject: [PATCH 26/31] Guarantee trailing tab title delivery --- docs/components/terminal.md | 6 ++ .../Terminal/Models/TerminalTabManager.swift | 65 +++++++++++++++++-- ...WorktreeTerminalState+AgentDetection.swift | 7 +- .../Models/WorktreeTerminalState.swift | 10 ++- supacode/Support/Debouncer.swift | 2 +- supacodeTests/TabTitleFlushRefreshTests.swift | 29 ++++++++- .../TerminalTabTitleCoalescingTests.swift | 38 +++++++++++ 7 files changed, 140 insertions(+), 17 deletions(-) diff --git a/docs/components/terminal.md b/docs/components/terminal.md index 22f5e6e8..4982a125 100644 --- a/docs/components/terminal.md +++ b/docs/components/terminal.md @@ -68,6 +68,12 @@ A tab's displayed title is, in order of precedence: 2. the **live shell title** the running program emits (OSC 2), else 3. an auto-generated default like `project 1`, `project 2`. +Rapid live-title animation is coalesced per tab to at most one visible update +per second so one spinner frame does not rebuild the entire tab bar. The newest +withheld title is applied at the end of that interval even when no agent is +running, so a live title may visibly lag by up to one second but its final value +is not left behind. + The Run Script tab is labeled **RUN SCRIPT** and is **title-locked** for its lifetime. Prowl also "learns" your shell's idle prompt so it doesn't mistake it for a meaningful title. diff --git a/supacode/Features/Terminal/Models/TerminalTabManager.swift b/supacode/Features/Terminal/Models/TerminalTabManager.swift index 4a1757ae..c3565224 100644 --- a/supacode/Features/Terminal/Models/TerminalTabManager.swift +++ b/supacode/Features/Terminal/Models/TerminalTabManager.swift @@ -34,6 +34,14 @@ final class TerminalTabManager { /// The most recent title held back by coalescing. A spinner that stops leaves /// no further change to carry it, so `flushPendingTitles` lands it instead. @ObservationIgnored private var pendingLiveTitles: [TerminalTabID: String] = [:] + @ObservationIgnored private let titleFlushClock: any Clock + @ObservationIgnored private var pendingTitleFlushTask: Task? + @ObservationIgnored private var scheduledPendingTitleFlushDate: Date? + @ObservationIgnored var onCoalescedTitlesFlushed: (([TerminalTabID]) -> Void)? + + init(titleFlushClock: any Clock = ContinuousClock()) { + self.titleFlushClock = titleFlushClock + } /// Creates a tab next to the current selection. With `select: false` the /// selection is left untouched (background creation, e.g. a headless handoff @@ -71,8 +79,14 @@ final class TerminalTabManager { guard let index = tabs.firstIndex(where: { $0.id == id }) else { return false } guard !tabs[index].isTitleLocked else { return false } // A TUI re-emits the same title constantly; skip the no-op write so it - // doesn't invalidate the tab bar while an agent streams output. - guard tabs[index].title != title else { return false } + // doesn't invalidate the tab bar while an agent streams output. The latest + // title still supersedes a held frame: A → B (held) → A must not flush B. + guard tabs[index].title != title else { + if pendingLiveTitles.removeValue(forKey: id) != nil { + scheduleNextPendingTitleFlush(referenceDate: now) + } + return false + } // A changed title arriving inside the interval is almost always the next // frame of an animation. Hold it rather than rebuilding the tab bar for it; // the newest one wins, so nothing queues up. @@ -80,23 +94,26 @@ final class TerminalTabManager { now.timeIntervalSince(lastWriteAt) < Self.liveTitleCoalescingInterval { pendingLiveTitles[id] = title + scheduleNextPendingTitleFlush(referenceDate: now) return false } - return writeLiveTitle(title, toTabAt: index, id: id, now: now) + let changed = writeLiveTitle(title, toTabAt: index, id: id, now: now) + scheduleNextPendingTitleFlush(referenceDate: now) + return changed } /// Lands any held-back title whose interval has elapsed. Returns the tabs whose /// visible `displayTitle` moved, so callers can refresh derived UI exactly as /// they would after `updateTitle`. /// - /// Driven by the agent-detection poll rather than a timer: the only way a - /// pending title is left stranded is that the writes stopped, and the poll is - /// already running for precisely the panes that animate. + /// A clock-driven trailing task calls this at the earliest pending deadline, + /// independently of agent detection, so non-agent programs cannot strand a + /// final title after the detection schedule goes cold. @discardableResult func flushPendingTitles(now: Date = Date()) -> [TerminalTabID] { guard !pendingLiveTitles.isEmpty else { return [] } var changed: [TerminalTabID] = [] - for (id, title) in pendingLiveTitles { + for (id, title) in Array(pendingLiveTitles) { guard let lastWriteAt = lastLiveTitleWriteAt[id], now.timeIntervalSince(lastWriteAt) >= Self.liveTitleCoalescingInterval else { continue } @@ -109,6 +126,7 @@ final class TerminalTabManager { changed.append(id) } } + scheduleNextPendingTitleFlush(referenceDate: now) return changed } @@ -129,6 +147,39 @@ final class TerminalTabManager { let liveIDs = Set(tabs.map(\.id)) lastLiveTitleWriteAt = lastLiveTitleWriteAt.filter { liveIDs.contains($0.key) } pendingLiveTitles = pendingLiveTitles.filter { liveIDs.contains($0.key) } + scheduleNextPendingTitleFlush(referenceDate: Date()) + } + + private func scheduleNextPendingTitleFlush(referenceDate: Date) { + let nextFlushDate = pendingLiveTitles.keys.compactMap { id in + lastLiveTitleWriteAt[id]?.addingTimeInterval(Self.liveTitleCoalescingInterval) + }.min() + guard let nextFlushDate else { + pendingTitleFlushTask?.cancel() + pendingTitleFlushTask = nil + scheduledPendingTitleFlushDate = nil + return + } + guard nextFlushDate != scheduledPendingTitleFlushDate else { return } + + pendingTitleFlushTask?.cancel() + scheduledPendingTitleFlushDate = nextFlushDate + let delay = max(0, nextFlushDate.timeIntervalSince(referenceDate)) + let sleep = titleFlushClock.anchoredSleep(for: .seconds(delay)) + pendingTitleFlushTask = Task { @MainActor [weak self] in + do { + try await sleep() + } catch { + return + } + guard let self, self.scheduledPendingTitleFlushDate == nextFlushDate else { return } + self.pendingTitleFlushTask = nil + self.scheduledPendingTitleFlushDate = nil + let changed = self.flushPendingTitles(now: nextFlushDate) + if !changed.isEmpty { + self.onCoalescedTitlesFlushed?(changed) + } + } } /// Every tab the coalescing bookkeeping still holds an entry for. diff --git a/supacode/Features/Terminal/Models/WorktreeTerminalState+AgentDetection.swift b/supacode/Features/Terminal/Models/WorktreeTerminalState+AgentDetection.swift index 2ae73c40..ec952440 100644 --- a/supacode/Features/Terminal/Models/WorktreeTerminalState+AgentDetection.swift +++ b/supacode/Features/Terminal/Models/WorktreeTerminalState+AgentDetection.swift @@ -26,7 +26,6 @@ extension WorktreeTerminalState { guard let self, let view, self.surfaces[view.id] != nil else { return } let hasAgent = await self.detectAgentState(for: view, tabId: tabId) let now = Date() - self.flushCoalescedTabTitles(now: now) let schedule = self.agentDetectionSchedules[view.id] ?? .cold self.agentDetectionSchedules[view.id] = hasAgent ? schedule.observedAgent(now: now) : schedule.observedNoAgent(now: now) @@ -270,10 +269,8 @@ extension WorktreeTerminalState { /// entries that follow them — the same refresh a title written directly through /// `updateTitle` triggers, so the two paths cannot drift. /// - /// Driven by the detection poll: a spinner that stops animating leaves no further - /// title change to carry its last frame, and the poll is already running for - /// exactly the panes that animate. Split out of that `Task` loop because the loop - /// offers no synchronous seam a test can drive. + /// The manager's clock-driven trailing flush uses the same refresh path. This + /// synchronous seam lets callers and tests force a flush at an explicit date. @discardableResult func flushCoalescedTabTitles(now: Date = Date()) -> [TerminalTabID] { let flushed = tabManager.flushPendingTitles(now: now) diff --git a/supacode/Features/Terminal/Models/WorktreeTerminalState.swift b/supacode/Features/Terminal/Models/WorktreeTerminalState.swift index ea133b34..9b1eb0f9 100644 --- a/supacode/Features/Terminal/Models/WorktreeTerminalState.swift +++ b/supacode/Features/Terminal/Models/WorktreeTerminalState.swift @@ -215,18 +215,24 @@ final class WorktreeTerminalState { worktree: Worktree, runSetupScript: Bool = false, defaultFontSize: Float32? = nil, - targetHandleRegistry: TerminalTargetHandleRegistry? = nil + targetHandleRegistry: TerminalTargetHandleRegistry? = nil, + titleFlushClock: any Clock = ContinuousClock() ) { self.runtime = runtime self.worktree = worktree self.targetHandleRegistry = targetHandleRegistry ?? TerminalTargetHandleRegistry() self.pendingSetupScript = runSetupScript self.defaultFontSize = defaultFontSize - self.tabManager = TerminalTabManager() + self.tabManager = TerminalTabManager(titleFlushClock: titleFlushClock) _repositorySettings = SharedReader( wrappedValue: RepositorySettings.default, .repositorySettings(worktree.repositoryRootURL) ) + self.tabManager.onCoalescedTitlesFlushed = { [weak self] tabIDs in + for tabID in tabIDs { + self?.refreshAgentEntriesForTitleChange(in: tabID) + } + } } var worktreeID: Worktree.ID { worktree.id } diff --git a/supacode/Support/Debouncer.swift b/supacode/Support/Debouncer.swift index b27a4386..fab0bc7c 100644 --- a/supacode/Support/Debouncer.swift +++ b/supacode/Support/Debouncer.swift @@ -51,7 +51,7 @@ final class Debouncer { /// `TestClock`, let the test advance past a deadline that was never armed, /// hanging the sleep forever (the CI-only `DebouncerTests` flake). extension Clock where Duration == Swift.Duration { - fileprivate func anchoredSleep(for interval: Duration) -> @Sendable () async throws -> Void { + func anchoredSleep(for interval: Duration) -> @Sendable () async throws -> Void { let deadline = now.advanced(by: interval) return { try await self.sleep(until: deadline, tolerance: nil) } } diff --git a/supacodeTests/TabTitleFlushRefreshTests.swift b/supacodeTests/TabTitleFlushRefreshTests.swift index f41fba4d..15cc8333 100644 --- a/supacodeTests/TabTitleFlushRefreshTests.swift +++ b/supacodeTests/TabTitleFlushRefreshTests.swift @@ -1,4 +1,5 @@ import AppKit +import Clocks import Foundation import GhosttyKit import Testing @@ -64,13 +65,36 @@ struct TabTitleFlushRefreshTests { #expect(fixture.state.tabManager.tabs.first?.title == "⠋ building") } + @Test func aWithheldTitleFlushesWithoutAnAgentDetectionTask() async { + let clock = TestClock() + let fixture = makeFixture(titleFlushClock: clock) + var received: [ActiveAgentEntry] = [] + fixture.state.onAgentEntryChanged = { received.append($0) } + + _ = fixture.state.tabManager.updateTitle(fixture.tabId, title: "A", now: start) + _ = fixture.state.tabManager.updateTitle( + fixture.tabId, + title: "B", + now: start.addingTimeInterval(0.2) + ) + #expect(fixture.state.agentDetectionTasks.isEmpty) + + await clock.advance(by: .seconds(1)) + for _ in 0..<10 { await Task.yield() } + + #expect(fixture.state.tabManager.tabs.first?.title == "B") + #expect(received.map(\.paneTitle) == ["B"]) + } + private struct Fixture { let state: WorktreeTerminalState let tabId: TerminalTabID let pane: GhosttySurfaceView } - private func makeFixture() -> Fixture { + private func makeFixture( + titleFlushClock: any Clock = ContinuousClock() + ) -> Fixture { let state = WorktreeTerminalState( runtime: GhosttyRuntime(), worktree: Worktree( @@ -79,7 +103,8 @@ struct TabTitleFlushRefreshTests { detail: "", workingDirectory: URL(fileURLWithPath: "/tmp/repo/worktree"), repositoryRootURL: URL(fileURLWithPath: "/tmp/repo") - ) + ), + titleFlushClock: titleFlushClock ) let pane = GhosttySurfaceView( runtime: state.runtime, diff --git a/supacodeTests/TerminalTabTitleCoalescingTests.swift b/supacodeTests/TerminalTabTitleCoalescingTests.swift index c8eca9f9..537b3d34 100644 --- a/supacodeTests/TerminalTabTitleCoalescingTests.swift +++ b/supacodeTests/TerminalTabTitleCoalescingTests.swift @@ -1,3 +1,4 @@ +import Clocks import Foundation import Testing @@ -91,6 +92,16 @@ struct TerminalTabTitleCoalescingTests { #expect(title(of: manager, id) == "finished") } + @Test func aSuppressedTitleIsDiscardedWhenTheLatestTitleRevertsToTheVisibleValue() { + let (manager, id) = makeManager() + _ = manager.updateTitle(id, title: "A", now: start) + _ = manager.updateTitle(id, title: "B", now: start.addingTimeInterval(0.2)) + + #expect(manager.updateTitle(id, title: "A", now: start.addingTimeInterval(0.4)) == false) + #expect(manager.flushPendingTitles(now: start.addingTimeInterval(1.5)).isEmpty) + #expect(title(of: manager, id) == "A") + } + @Test func coalescingIsPerTabNotGlobal() { let manager = TerminalTabManager() let first = manager.createTab(title: "first", icon: nil) @@ -104,6 +115,33 @@ struct TerminalTabTitleCoalescingTests { #expect(title(of: manager, second) == "⠋ two") } + @Test func automaticFlushRearmsForTheNextTabsLaterDeadline() async { + let clock = TestClock() + let manager = TerminalTabManager(titleFlushClock: clock) + let first = manager.createTab(title: "first", icon: nil) + let second = manager.createTab(title: "second", icon: nil) + var flushed: [[TerminalTabID]] = [] + manager.onCoalescedTitlesFlushed = { flushed.append($0) } + + _ = manager.updateTitle(first, title: "first A", now: start) + _ = manager.updateTitle(first, title: "first B", now: start.addingTimeInterval(0.1)) + _ = manager.updateTitle(second, title: "second A", now: start.addingTimeInterval(0.4)) + _ = manager.updateTitle(second, title: "second B", now: start.addingTimeInterval(0.5)) + + await clock.advance(by: .seconds(0.9)) + for _ in 0..<10 { await Task.yield() } + + #expect(title(of: manager, first) == "first B") + #expect(title(of: manager, second) == "second A") + #expect(flushed == [[first]]) + + await clock.advance(by: .seconds(0.4)) + for _ in 0..<10 { await Task.yield() } + + #expect(title(of: manager, second) == "second B") + #expect(flushed == [[first], [second]]) + } + @Test func aLockedTitleIsNeverWrittenOrHeld() { let manager = TerminalTabManager() let id = manager.createTab(title: "pinned", icon: nil, isTitleLocked: true) -- 2.51.2 From 61a59cc5d5f2e3e3932e23b637e59e5e3a86c206 Mon Sep 17 00:00:00 2001 From: onevcat Date: Sun, 2 Aug 2026 03:04:52 +0900 Subject: [PATCH 27/31] Document tab title coalescing review --- .../000-plan.md | 7 ++- .../006-tab-title-coalescing.md | 63 +++++++++++++++++++ 2 files changed, 68 insertions(+), 2 deletions(-) create mode 100644 docs-ai/056-performance-optimization-2026-08/006-tab-title-coalescing.md diff --git a/docs-ai/056-performance-optimization-2026-08/000-plan.md b/docs-ai/056-performance-optimization-2026-08/000-plan.md index f7b25f77..f61c9e9c 100644 --- a/docs-ai/056-performance-optimization-2026-08/000-plan.md +++ b/docs-ai/056-performance-optimization-2026-08/000-plan.md @@ -4,7 +4,7 @@ | --- | --- | | **Status** | Implemented | | **Anchor date** | 2026-08-01 | -| **Primary PRs** | #644–#646, #652, #653; review queue #647–#650 | +| **Primary PRs** | #644–#646, #649, #652, #653; review queue #647, #648, #650 | | **Related** | [030-agent-status-detection](../030-agent-status-detection/000-plan.md), [032-performance-hardening](../032-performance-hardening/000-plan.md), [037-line-diff-tracking](../037-line-diff-tracking/000-plan.md), `docs/components/diff-view.md` | ## Background @@ -137,7 +137,7 @@ pending independent code-path and test review. | #646 | Memoize per-surface agent screen parsing when agent and visible text are unchanged | Merged: exact agent/text cache identity preserves raw-state semantics; stabilization still runs per tick; cache lifetime follows detection/surface cleanup | `b2ac2936` | | #647 | Deduplicate raw-state-only agent emissions and narrow sidebar invalidation | Decide whether stale CLI `raw_state` is acceptable; trace UI/CLI ownership before merge | `e77ba660` | | #648 | Replace per-agent worktree scans/path resolution with a cached directory index | Verify deepest-match and symlink semantics, cache invalidation, render-path purity, and current CI | `08773383` | -| #649 | Coalesce animated terminal-title writes and remove quadratic tab lookup | Verify final-title delivery, close/prune lifecycle, custom/locked titles, and clock boundaries | `b77888f3` | +| #649 | Coalesce animated terminal-title writes and remove quadratic tab lookup | Reviewed for fork integration: per-tab coalescing now has clock-driven trailing delivery independent of agent detection, stale pending frames are discarded, and the optimized tab map is rebuilt after every structural mutation | `5c7e2a35` | | #650 | Cache parsed transcript tails and fast-path fingerprint normalization | Verify append/mtime invalidation, cache pruning, Unicode equivalence, collision behavior, and current CI | `c97cbb4` | ## Alternatives & decisions @@ -173,3 +173,6 @@ pending independent code-path and test review. [002-opt-in-debug-tca-action-logging.md](002-opt-in-debug-tca-action-logging.md). - Updated 2026-08-02: Merged per-surface agent screen-scan memoization from #646 — see [003-agent-screen-scan-memoization.md](003-agent-screen-scan-memoization.md). +- Updated 2026-08-02: Reviewed animated terminal-title coalescing from #649 and prepared + fork integration with guaranteed trailing delivery — see + [006-tab-title-coalescing.md](006-tab-title-coalescing.md). diff --git a/docs-ai/056-performance-optimization-2026-08/006-tab-title-coalescing.md b/docs-ai/056-performance-optimization-2026-08/006-tab-title-coalescing.md new file mode 100644 index 00000000..36998830 --- /dev/null +++ b/docs-ai/056-performance-optimization-2026-08/006-tab-title-coalescing.md @@ -0,0 +1,63 @@ +# 056.006 — Terminal Tab Title Coalescing + +## Context + +Agent TUIs commonly animate a spinner in the terminal title through OSC 2 updates at roughly +10 Hz. `TerminalTabManager.tabs` is observed as one array, so every visible title write rebuilds +all tab views and drives AppKit layout and Core Animation work across the window. The original +implementation also resolved each row's tab with a linear search, making one tab-bar rebuild +quadratic in the number of tabs. + +The author sampled a 300-second workload with 32 tabs and attributed 82.5% of one main-thread +core to SwiftUI graph work, Core Animation commits, and AppKit layout. The review verified the +structural invalidation path and the resulting behavior; it did not independently reproduce that +exact profile. + +## Change + +- #649 builds one tab lookup dictionary for each tab-bar render instead of scanning the tab + array for every row. +- Live terminal-title writes are coalesced independently per tab, with at most one visible write + per second. Only the newest withheld title is retained, so bursts do not create queues. +- A clock-driven trailing task flushes the newest withheld title at its deadline even when agent + detection is absent or has moved to its cold schedule. The first implementation tied delivery + to the detection poll and could leave the final title stale indefinitely for a non-agent + program. +- If a title sequence returns to its currently visible value, the obsolete withheld frame is + discarded. This prevents `A -> B (held) -> A` from later flashing the stale `B` value. +- Closing or pruning a tab removes its coalescing bookkeeping. Custom titles continue to mask + live values while preserving the newest underlying title, and title-locked tabs remain + immutable. +- A trailing flush refreshes Active Agents through the same title-change path as an immediate + write. User-facing terminal documentation records the one-second maximum visible lag. + +## Refs + +- Original PR #649 +- Fork integration PR pending +- Original implementation commits `14a6c1d8` and `b77888f3` +- Fork follow-up commit `5c7e2a35` + +## Current state + +Animated live titles no longer mutate the observed tab array on every frame. Each tab has an +independent deadline, and the scheduled task always arms the earliest one, flushes all entries +that are due, then re-arms for the next later deadline. Cancellation and a stored-deadline guard +prevent a replaced or reverted task from applying stale state. + +The coalescing boundary is intentionally limited to the shared tab title. An individual pane's +raw title remains available to split-pane and CLI consumers, preserving the existing semantic +distinction between a tab title and a surface title. + +## Verification + +- The original title-coalescing, flush-refresh, and pane-title suites passed 21 tests before the + follow-up. +- Regression tests were added first for stale `A -> B -> A` delivery and trailing delivery with + no agent-detection task; each failed against the original implementation before its fix. +- A clock-driven two-tab test covers re-arming after the first deadline so a later pending title + cannot be stranded. +- The affected title, Active Agents, and terminal-manager suites passed 45 tests before the final + multi-tab addition; the new test also passed independently. +- `make check` passed before integrating the latest `main`. +- `make build-app` completed with no errors or warnings before integrating the latest `main`. -- 2.51.2 From 34d17aaddf5e1c986bbe6a5d2d9f1550acdd0828 Mon Sep 17 00:00:00 2001 From: onevcat Date: Sun, 2 Aug 2026 03:07:30 +0900 Subject: [PATCH 28/31] Link tab title integration PR --- docs-ai/056-performance-optimization-2026-08/000-plan.md | 2 +- .../006-tab-title-coalescing.md | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/docs-ai/056-performance-optimization-2026-08/000-plan.md b/docs-ai/056-performance-optimization-2026-08/000-plan.md index f61c9e9c..fcbf10e2 100644 --- a/docs-ai/056-performance-optimization-2026-08/000-plan.md +++ b/docs-ai/056-performance-optimization-2026-08/000-plan.md @@ -4,7 +4,7 @@ | --- | --- | | **Status** | Implemented | | **Anchor date** | 2026-08-01 | -| **Primary PRs** | #644–#646, #649, #652, #653; review queue #647, #648, #650 | +| **Primary PRs** | #644–#646, #649, #652, #653, #656; review queue #647, #648, #650 | | **Related** | [030-agent-status-detection](../030-agent-status-detection/000-plan.md), [032-performance-hardening](../032-performance-hardening/000-plan.md), [037-line-diff-tracking](../037-line-diff-tracking/000-plan.md), `docs/components/diff-view.md` | ## Background diff --git a/docs-ai/056-performance-optimization-2026-08/006-tab-title-coalescing.md b/docs-ai/056-performance-optimization-2026-08/006-tab-title-coalescing.md index 36998830..823e085c 100644 --- a/docs-ai/056-performance-optimization-2026-08/006-tab-title-coalescing.md +++ b/docs-ai/056-performance-optimization-2026-08/006-tab-title-coalescing.md @@ -34,7 +34,7 @@ exact profile. ## Refs - Original PR #649 -- Fork integration PR pending +- Fork integration PR #656 - Original implementation commits `14a6c1d8` and `b77888f3` - Fork follow-up commit `5c7e2a35` -- 2.51.2 From 2c2eeddf5afcda6e82e7791ecfe5f11db05a25a9 Mon Sep 17 00:00:00 2001 From: onevcat Date: Sun, 2 Aug 2026 03:24:45 +0900 Subject: [PATCH 29/31] Bound transcript fragment reuse --- .../AgentDetection/AgentSessionResolver.swift | 88 ++++++++++++----- ...gentSessionFingerprintNormalizeTests.swift | 14 +++ .../TranscriptFragmentCacheTests.swift | 94 +++++++++++++++---- 3 files changed, 155 insertions(+), 41 deletions(-) diff --git a/supacode/Infrastructure/AgentDetection/AgentSessionResolver.swift b/supacode/Infrastructure/AgentDetection/AgentSessionResolver.swift index a4caf94e..90cec251 100644 --- a/supacode/Infrastructure/AgentDetection/AgentSessionResolver.swift +++ b/supacode/Infrastructure/AgentDetection/AgentSessionResolver.swift @@ -70,22 +70,40 @@ nonisolated struct AgentSessionResolution: Sendable { let isFresh: Bool } -/// Parsed, normalized transcript fragments reused across resolver polls. +/// Parsed, normalized transcript fragments reused across resolver polls and panes. /// /// Fingerprint matching re-reads the same transcript tails every time a pane's /// session cache expires (5 s while a session stays resolved). The bytes are /// almost always identical and the tails sit in the page cache, so the cost is /// re-parsing JSON and re-normalizing text rather than I/O. Keying the parsed /// result on the file's modification time replays it for unchanged transcripts -/// without altering which candidate wins. +/// without altering which candidate wins. Entry-count and retained-payload limits +/// bound process churn and candidate-set churn independently of process cleanup. nonisolated struct TranscriptFragmentCache: Sendable { struct Key: Hashable, Sendable { let path: String let modifiedAt: Date } - private var entries: [Key: [String]] = [:] - private var consulted: Set = [] + private struct Entry: Sendable { + let fragments: [String] + let retainedUTF8Bytes: Int + var lastAccess: UInt64 + } + + private var entries: [Key: Entry] = [:] + private var retainedUTF8Bytes = 0 + private var accessCounter: UInt64 = 0 + private let maxEntryCount: Int + private let maxRetainedUTF8Bytes: Int + + init( + maxEntryCount: Int = 128, + maxRetainedUTF8Bytes: Int = 8 * 1_024 * 1_024 + ) { + self.maxEntryCount = max(0, maxEntryCount) + self.maxRetainedUTF8Bytes = max(0, maxRetainedUTF8Bytes) + } var count: Int { entries.count } @@ -93,20 +111,49 @@ nonisolated struct TranscriptFragmentCache: Sendable { /// `load()`. A nil `load()` is deliberately not cached: an unreadable tail is /// transient, and caching the failure would keep a recovered file excluded. mutating func fragments(for key: Key, load: () -> [String]?) -> [String]? { - consulted.insert(key) - if let cached = entries[key] { return cached } + accessCounter &+= 1 + if var cached = entries[key] { + cached.lastAccess = accessCounter + entries[key] = cached + return cached.fragments + } + // Only the newest observed version of a path can be useful. Removing older + // modification-time keys immediately avoids spending the shared budget on + // append history until LRU pressure happens to arrive. Do this even when + // the new version is temporarily unreadable: an old key cannot answer a + // request for the new file contents. + for superseded in entries.keys.filter({ $0.path == key.path }) { + remove(superseded) + } guard let loaded = load() else { return nil } - entries[key] = loaded + + let byteCount = + key.path.utf8.count + + loaded.reduce(into: 0) { count, fragment in + count += fragment.utf8.count + } + guard maxEntryCount > 0, byteCount <= maxRetainedUTF8Bytes else { return loaded } + entries[key] = Entry( + fragments: loaded, + retainedUTF8Bytes: byteCount, + lastAccess: accessCounter + ) + retainedUTF8Bytes += byteCount + evictIfNeeded() return loaded } - /// Drops every entry not consulted since the previous prune: superseded - /// versions of an appended transcript, and files that left the candidate set. - /// Because the key carries a modification date, an actively written transcript - /// mints a new entry per poll; without this the cache would grow without bound. - mutating func pruneUnconsulted() { - entries = entries.filter { consulted.contains($0.key) } - consulted.removeAll(keepingCapacity: true) + private mutating func evictIfNeeded() { + while entries.count > maxEntryCount || retainedUTF8Bytes > maxRetainedUTF8Bytes { + guard let leastRecentlyUsed = entries.min(by: { $0.value.lastAccess < $1.value.lastAccess })?.key + else { return } + remove(leastRecentlyUsed) + } + } + + private mutating func remove(_ key: Key) { + guard let removed = entries.removeValue(forKey: key) else { return } + retainedUTF8Bytes -= removed.retainedUTF8Bytes } } @@ -186,9 +233,10 @@ actor AgentSessionResolver { } private var cache: [CacheKey: CachedResult] = [:] - /// Per-pane transcript fragment reuse, kept beside `cache` so both are evicted - /// on the same liveness check. - private var fragmentCaches: [CacheKey: TranscriptFragmentCache] = [:] + /// Transcript parsing depends only on file identity, not on the process doing + /// the match. Sharing one bounded cache avoids retaining duplicate 128 KiB + /// tails for every pane that consults the same candidate set. + private var fragmentCache = TranscriptFragmentCache() private let fileManager: FileManager private let homeDirectory: URL @@ -226,7 +274,6 @@ actor AgentSessionResolver { } } - var fragments = fragmentCaches[key] ?? TranscriptFragmentCache() let (resolved, usedWideScan) = resolveUncached( ResolveRequest( identified: identified, @@ -236,10 +283,8 @@ actor AgentSessionResolver { configRoot: configRoot, now: now ), - fragments: &fragments + fragments: &fragmentCache ) - fragments.pruneUnconsulted() - fragmentCaches[key] = fragments var session = resolved var provisionalID: String? if let candidate = session, candidate.confidence == .medium { @@ -265,7 +310,6 @@ actor AgentSessionResolver { cache = cache.filter { entry in ProcessDetection.processStartDate(pid: entry.key.pid) == entry.key.startedAt } - fragmentCaches = fragmentCaches.filter { cache[$0.key] != nil } } return AgentSessionResolution(session: session, isFresh: true) } diff --git a/supacodeTests/AgentSessionFingerprintNormalizeTests.swift b/supacodeTests/AgentSessionFingerprintNormalizeTests.swift index c8ee1462..4406389f 100644 --- a/supacodeTests/AgentSessionFingerprintNormalizeTests.swift +++ b/supacodeTests/AgentSessionFingerprintNormalizeTests.swift @@ -91,6 +91,20 @@ struct AgentSessionFingerprintNormalizeTests { } } + @Test func fastPathMatchesReferenceForEveryASCIIByteAndPair() { + let scalars = (UInt8.min...UInt8.max).prefix(128).map { String(Unicode.Scalar($0)) } + for first in scalars { + #expect(AgentSessionFingerprintMatcher.normalize(first) == Self.pristine(first)) + for second in scalars { + let input = first + second + #expect( + AgentSessionFingerprintMatcher.normalize(input) == Self.pristine(input), + "normalize diverged for \(String(reflecting: input))" + ) + } + } + } + @Test func normalizesToExpectedText() { #expect(AgentSessionFingerprintMatcher.normalize(" \u{001B}[1;31mHello\t\tWORLD ") == "hello world") #expect(AgentSessionFingerprintMatcher.normalize("") == "") diff --git a/supacodeTests/TranscriptFragmentCacheTests.swift b/supacodeTests/TranscriptFragmentCacheTests.swift index 28dc3f73..7c95dea1 100644 --- a/supacodeTests/TranscriptFragmentCacheTests.swift +++ b/supacodeTests/TranscriptFragmentCacheTests.swift @@ -68,30 +68,86 @@ struct TranscriptFragmentCacheTests { #expect(recovered == ["now readable"]) } - @Test func pruneDropsOnlyEntriesNotConsultedSinceTheLastPrune() { + @Test func replacesAnOlderVersionOfTheSamePathImmediately() { var cache = TranscriptFragmentCache() - let stable = key("/tmp/stable.jsonl", 100) - let superseded = key("/tmp/busy.jsonl", 100) - _ = cache.fragments(for: stable) { ["stable"] } - _ = cache.fragments(for: superseded) { ["v1"] } - #expect(cache.count == 2) - - cache.pruneUnconsulted() - #expect(cache.count == 2, "Both were consulted in this round, so both survive") - - // Next round: the busy transcript is appended to, so its old key is never - // consulted again and must not accumulate. - _ = cache.fragments(for: stable) { ["stable"] } + _ = cache.fragments(for: key("/tmp/busy.jsonl", 100)) { ["v1"] } _ = cache.fragments(for: key("/tmp/busy.jsonl", 101)) { ["v2"] } - cache.pruneUnconsulted() - #expect(cache.count == 2) + #expect(cache.count == 1) + } + + @Test func dropsAnOlderVersionWhenLoadingTheNewVersionFails() { + var cache = TranscriptFragmentCache() + _ = cache.fragments(for: key("/tmp/busy.jsonl", 100)) { ["v1"] } + + #expect(cache.fragments(for: key("/tmp/busy.jsonl", 101)) { nil } == nil) + #expect(cache.count == 0) + } + + @Test func evictsTheLeastRecentlyUsedEntryAtCapacity() { + var cache = TranscriptFragmentCache(maxEntryCount: 2, maxRetainedUTF8Bytes: .max) + let first = key("/tmp/first.jsonl", 100) + let second = key("/tmp/second.jsonl", 100) + let third = key("/tmp/third.jsonl", 100) + _ = cache.fragments(for: first) { ["first"] } + _ = cache.fragments(for: second) { ["second"] } + _ = cache.fragments(for: first) { ["not reached"] } + _ = cache.fragments(for: third) { ["third"] } + + var firstReloads = 0 + _ = cache.fragments(for: first) { + firstReloads += 1 + return ["first reloaded"] + } + var secondReloads = 0 + _ = cache.fragments(for: second) { + secondReloads += 1 + return ["second reloaded"] + } + + #expect(firstReloads == 0, "A cache hit must make the entry most recently used") + #expect(secondReloads == 1, "The least recently used entry must be evicted first") + } + + @Test func doesNotRetainAnEntryLargerThanTheByteBudget() { + var cache = TranscriptFragmentCache(maxEntryCount: 10, maxRetainedUTF8Bytes: 3) + let target = key("/tmp/large.jsonl", 100) var loads = 0 - _ = cache.fragments(for: superseded) { - loads += 1 - return ["v1 again"] + + for _ in 0..<2 { + _ = cache.fragments(for: target) { + loads += 1 + return ["four"] + } + } + + #expect(loads == 2) + #expect(cache.count == 0) + } + + @Test func byteBudgetEvictsTheLeastRecentlyUsedEntry() { + var cache = TranscriptFragmentCache(maxEntryCount: 10, maxRetainedUTF8Bytes: 10) + let first = key("a", 100) + let second = key("b", 100) + let third = key("c", 100) + _ = cache.fragments(for: first) { ["1234"] } + _ = cache.fragments(for: second) { ["1234"] } + _ = cache.fragments(for: first) { ["not reached"] } + _ = cache.fragments(for: third) { ["1234"] } + + var firstReloads = 0 + _ = cache.fragments(for: first) { + firstReloads += 1 + return ["1234"] } - #expect(loads == 1, "The superseded entry was evicted, so it must reload") + var secondReloads = 0 + _ = cache.fragments(for: second) { + secondReloads += 1 + return ["1234"] + } + + #expect(firstReloads == 0) + #expect(secondReloads == 1) } @Test func bestMatchServesASecondCallFromTheCache() throws { -- 2.51.2 From 89652cf226e4e521467248839e2fb4ff44608bb1 Mon Sep 17 00:00:00 2001 From: onevcat Date: Sun, 2 Aug 2026 03:25:43 +0900 Subject: [PATCH 30/31] Document transcript cache review --- .../000-plan.md | 7 +- .../007-transcript-fragment-cache.md | 72 +++++++++++++++++++ 2 files changed, 77 insertions(+), 2 deletions(-) create mode 100644 docs-ai/056-performance-optimization-2026-08/007-transcript-fragment-cache.md diff --git a/docs-ai/056-performance-optimization-2026-08/000-plan.md b/docs-ai/056-performance-optimization-2026-08/000-plan.md index f7b25f77..c9dca76c 100644 --- a/docs-ai/056-performance-optimization-2026-08/000-plan.md +++ b/docs-ai/056-performance-optimization-2026-08/000-plan.md @@ -4,7 +4,7 @@ | --- | --- | | **Status** | Implemented | | **Anchor date** | 2026-08-01 | -| **Primary PRs** | #644–#646, #652, #653; review queue #647–#650 | +| **Primary PRs** | #644–#646, #650, #652, #653; review queue #647–#649 | | **Related** | [030-agent-status-detection](../030-agent-status-detection/000-plan.md), [032-performance-hardening](../032-performance-hardening/000-plan.md), [037-line-diff-tracking](../037-line-diff-tracking/000-plan.md), `docs/components/diff-view.md` | ## Background @@ -138,7 +138,7 @@ pending independent code-path and test review. | #647 | Deduplicate raw-state-only agent emissions and narrow sidebar invalidation | Decide whether stale CLI `raw_state` is acceptable; trace UI/CLI ownership before merge | `e77ba660` | | #648 | Replace per-agent worktree scans/path resolution with a cached directory index | Verify deepest-match and symlink semantics, cache invalidation, render-path purity, and current CI | `08773383` | | #649 | Coalesce animated terminal-title writes and remove quadratic tab lookup | Verify final-title delivery, close/prune lifecycle, custom/locked titles, and clock boundaries | `b77888f3` | -| #650 | Cache parsed transcript tails and fast-path fingerprint normalization | Verify append/mtime invalidation, cache pruning, Unicode equivalence, collision behavior, and current CI | `c97cbb4` | +| #650 | Cache parsed transcript tails and fast-path fingerprint normalization | Reviewed for fork integration: normalized fragments use one resolver-wide, entry- and payload-bounded LRU; the ASCII and escape-absence paths are pinned to the original Unicode-aware formulation | `c97cbb4` | ## Alternatives & decisions @@ -173,3 +173,6 @@ pending independent code-path and test review. [002-opt-in-debug-tca-action-logging.md](002-opt-in-debug-tca-action-logging.md). - Updated 2026-08-02: Merged per-surface agent screen-scan memoization from #646 — see [003-agent-screen-scan-memoization.md](003-agent-screen-scan-memoization.md). +- Updated 2026-08-02: Reviewed transcript-fragment reuse and fingerprint normalization from + #650 and bounded cache lifetime independently of process cleanup — see + [007-transcript-fragment-cache.md](007-transcript-fragment-cache.md). diff --git a/docs-ai/056-performance-optimization-2026-08/007-transcript-fragment-cache.md b/docs-ai/056-performance-optimization-2026-08/007-transcript-fragment-cache.md new file mode 100644 index 00000000..ced39129 --- /dev/null +++ b/docs-ai/056-performance-optimization-2026-08/007-transcript-fragment-cache.md @@ -0,0 +1,72 @@ +# 056.007 — Transcript Fragment Cache and Fingerprint Normalization + +## Context + +Agent session resolution periodically compares visible terminal text with recent transcript +tails. Before #650, every fresh resolution reread, parsed, normalized, and filtered the same +candidate tails even when their bytes had not changed. The tails were normally already in the +filesystem page cache, so repeated JSON parsing, ANSI stripping, Unicode case folding, and +whitespace normalization dominated the matching cost rather than file I/O. + +The author sampled a 20-pane instance and attributed 15.9% of one core to agent detection, +13.0% to `AgentSessionResolver.resolve`, 11.4% to fingerprint matching, and 6.7% to normalization. +The review verified the repeated computation and cache boundaries but did not independently +reproduce those exact percentages. + +## Change + +- #650 caches normalized, length-filtered fragments by transcript path and modification date. + An unchanged file reuses the exact values that the matcher would otherwise recompute; an + appended file receives a new key, and an unreadable tail is never cached as a failure. +- The fork follow-up replaces per-process fragment caches with one resolver-wide LRU. Fragment + parsing is a pure function of transcript bytes and does not depend on the consulting process, + agent profile, config root, candidate set, or active screen, so sharing removes duplicate tails + across panes without changing scoring. +- The shared cache retains at most 128 transcript entries and 8 MiB of normalized UTF-8 payload. + Both limits are independent of process-cache cleanup, so ordinary process churn cannot retain + one multi-megabyte cache per dead process. A single entry larger than the payload budget is + returned to the current match but not retained. +- Observing a new modification date removes older cached versions of the same path immediately, + including when the new tail is temporarily unreadable. Entry and byte-budget pressure evict + the least recently used value. +- Fingerprint normalization skips the ANSI regex when the input contains no ESC byte. Fully ASCII + strings use a byte scanner for CSI removal, ASCII case folding, and whitespace collapse; any + non-ASCII byte falls back to the original Unicode-aware formulation. + +## Refs + +- Original PR #650 +- Fork integration PR pending +- Original implementation commits `1e8933cb` and `c97cbb4` +- Fork follow-up commit `2c2eeddf` + +## Current state + +The resolver result cache still controls how often each process performs a fresh match. On a +fresh match, candidates consult the shared fragment LRU, so several panes scanning the same +history reuse one normalized value instead of retaining duplicates. Cache eviction can reduce the +optimization to the pre-cache computation cost under an unusually broad, disjoint candidate set, +but it cannot alter which candidate wins. + +The 8 MiB limit measures normalized UTF-8 payload plus path bytes, not total allocator RSS. The +128-entry limit separately bounds array and string-object overhead. Transcript paths normally +reside on APFS, whose modification-time precision makes same-path/same-time content collisions +impractical for the append-only transcript workflow; deliberately preserving timestamps while +rewriting content is outside this cache contract. + +## Verification + +- The original normalization, fragment-cache, profile, and resolver suites passed 54 tests before + the follow-up. +- Entry-count and byte-budget tests were added first and failed to compile until the cache exposed + bounded construction. A new-version load-failure test then failed against the first LRU draft + until superseded versions were removed before loading. +- Tests cover unchanged-file reuse, modification-date invalidation, unreadable-tail recovery, + immediate same-path replacement, entry-count LRU, cumulative byte-budget LRU, oversized values, + and matcher replay after the source files disappear. +- The normalization fast path is compared with a pristine copy of the original implementation + across a hand-built Unicode/CSI corpus, 2,000 deterministic random ASCII strings, and every + one- and two-byte ASCII combination. +- The final four focused suites passed 59 tests with no failures or warnings. +- `make check` passed before integrating the latest `main`. +- `make build-app` completed with no errors or warnings before integrating the latest `main`. -- 2.51.2 From 5481158c79af4a6e2ce389129074022481ee854c Mon Sep 17 00:00:00 2001 From: onevcat Date: Sun, 2 Aug 2026 03:28:29 +0900 Subject: [PATCH 31/31] Link transcript cache integration PR --- docs-ai/056-performance-optimization-2026-08/000-plan.md | 2 +- .../007-transcript-fragment-cache.md | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/docs-ai/056-performance-optimization-2026-08/000-plan.md b/docs-ai/056-performance-optimization-2026-08/000-plan.md index c9dca76c..8ea22ebf 100644 --- a/docs-ai/056-performance-optimization-2026-08/000-plan.md +++ b/docs-ai/056-performance-optimization-2026-08/000-plan.md @@ -4,7 +4,7 @@ | --- | --- | | **Status** | Implemented | | **Anchor date** | 2026-08-01 | -| **Primary PRs** | #644–#646, #650, #652, #653; review queue #647–#649 | +| **Primary PRs** | #644–#646, #650, #652, #653, #657; review queue #647–#649 | | **Related** | [030-agent-status-detection](../030-agent-status-detection/000-plan.md), [032-performance-hardening](../032-performance-hardening/000-plan.md), [037-line-diff-tracking](../037-line-diff-tracking/000-plan.md), `docs/components/diff-view.md` | ## Background diff --git a/docs-ai/056-performance-optimization-2026-08/007-transcript-fragment-cache.md b/docs-ai/056-performance-optimization-2026-08/007-transcript-fragment-cache.md index ced39129..a8323683 100644 --- a/docs-ai/056-performance-optimization-2026-08/007-transcript-fragment-cache.md +++ b/docs-ai/056-performance-optimization-2026-08/007-transcript-fragment-cache.md @@ -36,7 +36,7 @@ reproduce those exact percentages. ## Refs - Original PR #650 -- Fork integration PR pending +- Fork integration PR #657 - Original implementation commits `1e8933cb` and `c97cbb4` - Fork follow-up commit `2c2eeddf`