From b54accbd4b800201edde065d5315c6ba450e60c0 Mon Sep 17 00:00:00 2001 From: Simon Heimlicher Date: Mon, 27 Jul 2026 21:13:18 +0200 Subject: [PATCH 01/10] Scan bytes with memchr instead of reducing over Data MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Sampling a running instance for 300 s put roughly one whole core — about 72% of everything the process burned — in GitClient.countLines, spread across seven com.apple.root.user-init dispatch threads: 31.16% Sequence.reduce(_:_:) 29.92% Data.Iterator.next() 15.04% closure #1 in static GitClient.countLines(in:) 11.22% ValueWitnesses 10.56% FixedSizeBufferValueWitnesses Data conforms to Sequence, so reduce and contains walk it through Data.Iterator with a value-witness call per byte — 65,536 of them per 64 KiB chunk. memchr covers the same bytes in one vectorized pass over contiguous memory. Measured on a 66 MB file: 278.0 ms -> 6.2 ms, 44.6x, with both counting 302,570 newlines. Semantics are unchanged and the eight existing countLinesInFiles tests pass untouched. Adds coverage for what a scan rewrite could plausibly break and the single-chunk cases could not catch: a file spanning four chunks, a newline on the exact chunk boundary, a NUL past the 8 KiB binary probe still counting as text, and an empty file. --- supacode/Clients/Git/GitClient.swift | 40 ++++++++++++- supacodeTests/GitClientLineChangesTests.swift | 56 +++++++++++++++++++ 2 files changed, 94 insertions(+), 2 deletions(-) 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/10] 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/10] 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/10] 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 82b385608585669992d3f770c460d841129ad5b2 Mon Sep 17 00:00:00 2001 From: onevcat Date: Sun, 2 Aug 2026 00:28:12 +0900 Subject: [PATCH 05/10] 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 06/10] 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 07/10] 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 08/10] 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 09/10] 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 10/10] 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