From de1b3e93e5e49396b0f0b1bc36cda0f50deec4fb Mon Sep 17 00:00:00 2001 From: Simon Heimlicher Date: Fri, 2 Oct 2026 01:34:45 +0200 Subject: [PATCH 1/4] Fetch check counts for badges and list checks only when selected The batch query asked every pull request for up to 100 check contexts with names, timestamps, and URLs. It was cheap in points but costly for GitHub to resolve, and GitHub's secondary limits count that server time against an account other tools share. Every pull request now carries per-state check counts, which the badge, checks ring, status text, merge readiness, and command palette read. Only the selected worktree's pull request also lists each check, for the checks popover and the failing-check actions; the popover of any other worktree shows the counts and asks to select it. Both query builders share one field selection. --- .../Clients/Github/GithubCLIClient.swift | 165 +++++++---------- .../Clients/Github/GithubCLIModels.swift | 6 +- .../Github/GithubPullRequestStatusCheck.swift | 88 ++++++++- .../Github/PullRequestMergeReadiness.swift | 3 +- .../Reducer/CommandPaletteFeature.swift | 4 +- .../PullRequestRefreshCoordinator.swift | 29 ++- ...epositoriesFeature+GithubIntegration.swift | 11 +- .../Views/PullRequestChecksPopoverView.swift | 9 +- .../Views/PullRequestStatusButton.swift | 15 +- ...atchedPullRequestRefreshReducerTests.swift | 31 ++++ App/Tests/PullRequestCheckCountsTests.swift | 168 ++++++++++++++++++ .../PullRequestRefreshCoordinatorTests.swift | 30 ++++ docs-ai/028-pr-status-tracking/000-plan.md | 2 + .../007-check-counts.md | 49 +++++ docs/components/github-pull-requests.md | 3 + 15 files changed, 486 insertions(+), 127 deletions(-) create mode 100644 App/Tests/PullRequestCheckCountsTests.swift create mode 100644 docs-ai/028-pr-status-tracking/007-check-counts.md diff --git a/App/Sources/Clients/Github/GithubCLIClient.swift b/App/Sources/Clients/Github/GithubCLIClient.swift index c1d0ff5a..f6aa0b7d 100644 --- a/App/Sources/Clients/Github/GithubCLIClient.swift +++ b/App/Sources/Clients/Github/GithubCLIClient.swift @@ -426,7 +426,8 @@ nonisolated private func sanitizeCrossRepoRequests( owner: request.owner, repo: request.repo, branches: branches, - allowedHeadRepositories: request.allowedHeadRepositories + allowedHeadRepositories: request.allowedHeadRepositories, + detailBranches: Set(request.detailBranches.map { $0.trimmingCharacters(in: .whitespacesAndNewlines) }) ) ) } @@ -595,54 +596,7 @@ nonisolated private func makeCrossRepoBatchQuery( let selection = """ \(branchAlias): pullRequests(\(pullRequestsArgs)) { nodes { - number - title - state - additions - deletions - isDraft - reviewDecision - mergeable - mergeStateStatus - url - updatedAt - headRefName - baseRefName - commits { - totalCount - } - author { - login - } - headRepository { - name - owner { login } - } - mergeQueueEntry { - position - estimatedTimeToMerge - state - } - statusCheckRollup { - contexts(first: 100) { - nodes { - ... on CheckRun { - name - status - conclusion - startedAt - completedAt - detailsUrl - } - ... on StatusContext { - context - state - targetUrl - createdAt - } - } - } - } + \(pullRequestNodeFields(includeCheckDetails: request.detailBranches.contains(branch))) } } """ @@ -671,6 +625,70 @@ nonisolated private func makeCrossRepoBatchQuery( ) } +// The pull request fields every refresh reads. Each check is listed only when asked for: per-state +// counts carry the badge and merge readiness, while a list of up to 100 contexts per pull request +// is most of what a batched query costs GitHub to resolve. +nonisolated func pullRequestNodeFields(includeCheckDetails: Bool) -> String { + let checkDetails = + includeCheckDetails + ? """ + nodes { + ... on CheckRun { + name + status + conclusion + startedAt + completedAt + detailsUrl + } + ... on StatusContext { + context + state + targetUrl + createdAt + } + } + """ + : "" + return """ + number + title + state + additions + deletions + isDraft + reviewDecision + mergeable + mergeStateStatus + url + updatedAt + headRefName + baseRefName + commits { + totalCount + } + author { + login + } + headRepository { + name + owner { login } + } + mergeQueueEntry { + position + estimatedTimeToMerge + state + } + statusCheckRollup { + contexts\(includeCheckDetails ? "(first: 100)" : "") { + checkRunCountsByState { state count } + statusContextCountsByState { state count } + \(checkDetails) + } + } + """ +} + nonisolated private func rankCrossRepoPullRequests( pullRequestsByAlias: [String: GithubGraphQLPullRequestResponse.PullRequestConnection], aliasMap: [String: String], @@ -1180,54 +1198,7 @@ nonisolated private func makeBatchPullRequestsQuery( let selection = """ \(alias): pullRequests(first: 5, states: [OPEN, MERGED, CLOSED], headRefName: \"\(escapedBranch)\", \(orderBy)) { nodes { - number - title - state - additions - deletions - isDraft - reviewDecision - mergeable - mergeStateStatus - url - updatedAt - headRefName - baseRefName - commits { - totalCount - } - author { - login - } - headRepository { - name - owner { login } - } - mergeQueueEntry { - position - estimatedTimeToMerge - state - } - statusCheckRollup { - contexts(first: 100) { - nodes { - ... on CheckRun { - name - status - conclusion - startedAt - completedAt - detailsUrl - } - ... on StatusContext { - context - state - targetUrl - createdAt - } - } - } - } + \(pullRequestNodeFields(includeCheckDetails: false)) } } """ diff --git a/App/Sources/Clients/Github/GithubCLIModels.swift b/App/Sources/Clients/Github/GithubCLIModels.swift index 2e8a122d..0986df5f 100644 --- a/App/Sources/Clients/Github/GithubCLIModels.swift +++ b/App/Sources/Clients/Github/GithubCLIModels.swift @@ -148,17 +148,21 @@ nonisolated struct CrossRepoPullRequestRequest: Sendable, Hashable { let repo: String let branches: [String] let allowedHeadRepositories: Set + /// Branches whose pull request also lists each check, not only the per-state counts. + let detailBranches: Set init( owner: String, repo: String, branches: [String], - allowedHeadRepositories: Set? = nil + allowedHeadRepositories: Set? = nil, + detailBranches: Set = [] ) { self.owner = owner self.repo = repo self.branches = branches self.allowedHeadRepositories = allowedHeadRepositories ?? [RepoKey(owner: owner, repo: repo)] + self.detailBranches = detailBranches } var key: RepoKey { diff --git a/App/Sources/Clients/Github/GithubPullRequestStatusCheck.swift b/App/Sources/Clients/Github/GithubPullRequestStatusCheck.swift index f815431e..875b3fe6 100644 --- a/App/Sources/Clients/Github/GithubPullRequestStatusCheck.swift +++ b/App/Sources/Clients/Github/GithubPullRequestStatusCheck.swift @@ -82,28 +82,37 @@ nonisolated struct GithubPullRequestStatusCheck: Decodable, Equatable, Hashable } } +// The checks reported for a pull request's head commit. A refresh carries `counts` for every pull +// request and the individual `checks` only where they are shown; `breakdown` reads whichever is +// present. nonisolated struct GithubPullRequestStatusCheckRollup: Decodable, Equatable, Hashable { let checks: [GithubPullRequestStatusCheck] + let counts: PullRequestCheckBreakdown? - init(checks: [GithubPullRequestStatusCheck]) { + init(checks: [GithubPullRequestStatusCheck], counts: PullRequestCheckBreakdown? = nil) { self.checks = checks + self.counts = counts + } + + var breakdown: PullRequestCheckBreakdown { + counts ?? PullRequestCheckBreakdown(checks: checks) } init(from decoder: Decoder) throws { if let checks = try? [GithubPullRequestStatusCheck](from: decoder) { - self.checks = checks + self.init(checks: checks) return } let container = try decoder.container(keyedBy: CodingKeys.self) if let checks = try? container.decode([GithubPullRequestStatusCheck].self, forKey: .contexts) { - self.checks = checks + self.init(checks: checks) return } if let contexts = try? container.decode(GithubPullRequestStatusCheckContexts.self, forKey: .contexts) { - self.checks = contexts.nodes + self.init(checks: contexts.nodes ?? [], counts: contexts.breakdown) return } - self.checks = [] + self.init(checks: []) } private enum CodingKeys: String, CodingKey { @@ -112,10 +121,67 @@ nonisolated struct GithubPullRequestStatusCheckRollup: Decodable, Equatable, Has } nonisolated private struct GithubPullRequestStatusCheckContexts: Decodable, Equatable { - let nodes: [GithubPullRequestStatusCheck] + struct StateCount: Decodable, Equatable { + let state: String + let count: Int + } + + let nodes: [GithubPullRequestStatusCheck]? + let checkRunCountsByState: [StateCount]? + let statusContextCountsByState: [StateCount]? + + // Maps GitHub's per-state counts onto the same buckets `GithubPullRequestStatusCheck.checkState` + // assigns to an individual check, so counts and a fetched list always agree. + var breakdown: PullRequestCheckBreakdown? { + guard checkRunCountsByState != nil || statusContextCountsByState != nil else { + return nil + } + var passed = 0 + var failed = 0 + var inProgress = 0 + var expected = 0 + var skipped = 0 + for entry in checkRunCountsByState ?? [] { + switch entry.state.uppercased() { + case "SUCCESS", "NEUTRAL": + passed += entry.count + case "CANCELLED", "SKIPPED": + skipped += entry.count + case "FAILURE", "TIMED_OUT", "ACTION_REQUIRED", "STARTUP_FAILURE", "STALE": + failed += entry.count + default: + inProgress += entry.count + } + } + for entry in statusContextCountsByState ?? [] { + switch entry.state.uppercased() { + case "SUCCESS": + passed += entry.count + case "FAILURE", "ERROR": + failed += entry.count + case "EXPECTED": + expected += entry.count + default: + inProgress += entry.count + } + } + return PullRequestCheckBreakdown( + passed: passed, + failed: failed, + inProgress: inProgress, + expected: expected, + skipped: skipped + ) + } } -nonisolated struct PullRequestCheckBreakdown: Equatable { +extension GithubPullRequest { + nonisolated var checkBreakdown: PullRequestCheckBreakdown { + statusCheckRollup?.breakdown ?? PullRequestCheckBreakdown(checks: []) + } +} + +nonisolated struct PullRequestCheckBreakdown: Equatable, Hashable { let passed: Int let failed: Int let inProgress: Int @@ -149,6 +215,14 @@ nonisolated struct PullRequestCheckBreakdown: Equatable { return parts.joined(separator: ", ") } + init(passed: Int, failed: Int, inProgress: Int, expected: Int, skipped: Int) { + self.passed = passed + self.failed = failed + self.inProgress = inProgress + self.expected = expected + self.skipped = skipped + } + init(checks: [GithubPullRequestStatusCheck]) { var passed = 0 var failed = 0 diff --git a/App/Sources/Clients/Github/PullRequestMergeReadiness.swift b/App/Sources/Clients/Github/PullRequestMergeReadiness.swift index 85e12c59..e4a1e0d5 100644 --- a/App/Sources/Clients/Github/PullRequestMergeReadiness.swift +++ b/App/Sources/Clients/Github/PullRequestMergeReadiness.swift @@ -15,8 +15,7 @@ nonisolated struct PullRequestMergeReadiness: Equatable, Hashable { let mergeable = pullRequest.mergeable?.uppercased() let mergeStateStatus = pullRequest.mergeStateStatus?.uppercased() let reviewDecision = pullRequest.reviewDecision?.uppercased() - let checks = pullRequest.statusCheckRollup?.checks ?? [] - let breakdown = PullRequestCheckBreakdown(checks: checks) + let breakdown = pullRequest.checkBreakdown if mergeable == "CONFLICTING" || mergeStateStatus == "DIRTY" { self.blockingReason = .mergeConflicts diff --git a/App/Sources/Features/CommandPalette/Reducer/CommandPaletteFeature.swift b/App/Sources/Features/CommandPalette/Reducer/CommandPaletteFeature.swift index 6c4ad226..948fa327 100644 --- a/App/Sources/Features/CommandPalette/Reducer/CommandPaletteFeature.swift +++ b/App/Sources/Features/CommandPalette/Reducer/CommandPaletteFeature.swift @@ -857,7 +857,7 @@ private func pullRequestItems( ) -> [CommandPaletteItem] { let isOpen = pullRequest.state.uppercased() == "OPEN" let mergeReadiness = PullRequestMergeReadiness(pullRequest: pullRequest) - let breakdown = PullRequestCheckBreakdown(checks: pullRequest.statusCheckRollup?.checks ?? []) + let breakdown = pullRequest.checkBreakdown let canMerge = isOpen && !pullRequest.isDraft && !mergeReadiness.isBlocking var items: [CommandPaletteItem] = [ @@ -936,7 +936,7 @@ private func makeFailingPullRequestItems( ) -> [CommandPaletteItem] { let isOpen = pullRequest.state.uppercased() == "OPEN" let checks = pullRequest.statusCheckRollup?.checks ?? [] - let hasFailingChecks = PullRequestCheckBreakdown(checks: checks).failed > 0 + let hasFailingChecks = pullRequest.checkBreakdown.failed > 0 guard isOpen && hasFailingChecks else { return [] } let hasFailingCheckWithDetails = checks.contains { $0.checkState == .failure && $0.detailsUrl != nil } let leadingTier = pullRequest.isDraft ? 1 : 0 diff --git a/App/Sources/Features/Repositories/BusinessLogic/PullRequestRefreshCoordinator.swift b/App/Sources/Features/Repositories/BusinessLogic/PullRequestRefreshCoordinator.swift index 2c1db9a1..ce8ce728 100644 --- a/App/Sources/Features/Repositories/BusinessLogic/PullRequestRefreshCoordinator.swift +++ b/App/Sources/Features/Repositories/BusinessLogic/PullRequestRefreshCoordinator.swift @@ -11,6 +11,8 @@ final class PullRequestRefreshCoordinator { let accountOverride: GithubAccountOverride? let branches: [String] let worktreeIDs: [Worktree.ID] + /// Branches whose pull request also lists each check, such as the selected worktree's. + let detailBranches: [String] var owner: String { repositories.first?.owner ?? "" @@ -28,7 +30,8 @@ final class PullRequestRefreshCoordinator { repo: String, accountOverride: GithubAccountOverride?, branches: [String], - worktreeIDs: [Worktree.ID] + worktreeIDs: [Worktree.ID], + detailBranches: [String] = [] ) { self.init( repositoryID: repositoryID, @@ -37,7 +40,8 @@ final class PullRequestRefreshCoordinator { repositories: [GithubRemoteInfo(host: host, owner: owner, repo: repo)], accountOverride: accountOverride, branches: branches, - worktreeIDs: worktreeIDs + worktreeIDs: worktreeIDs, + detailBranches: detailBranches ) } @@ -48,7 +52,8 @@ final class PullRequestRefreshCoordinator { repositories: [GithubRemoteInfo], accountOverride: GithubAccountOverride?, branches: [String], - worktreeIDs: [Worktree.ID] + worktreeIDs: [Worktree.ID], + detailBranches: [String] = [] ) { self.repositoryID = repositoryID self.repositoryRootURL = repositoryRootURL @@ -57,6 +62,7 @@ final class PullRequestRefreshCoordinator { self.accountOverride = accountOverride self.branches = branches self.worktreeIDs = worktreeIDs + self.detailBranches = detailBranches } private static func deduplicateRepositories(_ repositories: [GithubRemoteInfo]) -> [GithubRemoteInfo] { @@ -126,7 +132,8 @@ final class PullRequestRefreshCoordinator { repositories: request.repositories.filter { $0.host == request.host }, accountOverride: request.accountOverride, branches: cleanedBranches, - worktreeIDs: request.worktreeIDs + worktreeIDs: request.worktreeIDs, + detailBranches: request.detailBranches ) guard !normalized.repositories.isEmpty else { return @@ -178,6 +185,10 @@ final class PullRequestRefreshCoordinator { for repository in request.repositories where seenRepositories.insert(repository.key).inserted { combinedRepositories.append(repository) } + var combinedDetailBranches = existing.detailBranches + for branch in request.detailBranches where !combinedDetailBranches.contains(branch) { + combinedDetailBranches.append(branch) + } hostBucket[request.repositoryID] = Request( repositoryID: request.repositoryID, repositoryRootURL: request.repositoryRootURL, @@ -185,7 +196,8 @@ final class PullRequestRefreshCoordinator { repositories: combinedRepositories, accountOverride: request.accountOverride, branches: combined, - worktreeIDs: workCombined + worktreeIDs: workCombined, + detailBranches: combinedDetailBranches ) } else { hostBucket[request.repositoryID] = request @@ -221,7 +233,8 @@ final class PullRequestRefreshCoordinator { owner: group.key.owner, repo: group.key.repo, branches: group.branches, - allowedHeadRepositories: group.allowedHeadRepositories + allowedHeadRepositories: group.allowedHeadRepositories, + detailBranches: group.detailBranches ) } do { @@ -400,6 +413,7 @@ final class PullRequestRefreshCoordinator { for repository in request.repositories { groupsByKey[repository.key, default: RepoRequestGroup(key: repository.key)].append( branches: request.branches, + detailBranches: request.detailBranches, allowedHeadRepositories: allowedHeadRepositories ) } @@ -411,6 +425,7 @@ final class PullRequestRefreshCoordinator { let key: RepoKey private(set) var branches: [String] = [] private(set) var allowedHeadRepositories: Set = [] + private(set) var detailBranches: Set = [] private var seenBranches: Set = [] init(key: RepoKey) { @@ -419,11 +434,13 @@ final class PullRequestRefreshCoordinator { mutating func append( branches newBranches: [String], + detailBranches newDetailBranches: [String], allowedHeadRepositories newAllowedHeadRepositories: Set ) { for branch in newBranches where seenBranches.insert(branch).inserted { branches.append(branch) } + detailBranches.formUnion(newDetailBranches) allowedHeadRepositories.formUnion(newAllowedHeadRepositories) } } diff --git a/App/Sources/Features/Repositories/Reducer/RepositoriesFeature+GithubIntegration.swift b/App/Sources/Features/Repositories/Reducer/RepositoriesFeature+GithubIntegration.swift index b47e187a..3415ac37 100644 --- a/App/Sources/Features/Repositories/Reducer/RepositoriesFeature+GithubIntegration.swift +++ b/App/Sources/Features/Repositories/Reducer/RepositoriesFeature+GithubIntegration.swift @@ -118,11 +118,14 @@ extension RepositoriesFeature { return .none } state.inFlightPullRequestRefreshRepositoryIDs.insert(repositoryID) + // Only the selected worktree's pull request lists each check; the rest carry counts. + let detailBranches = worktrees.filter { $0.id == state.selectedWorktreeID }.map(\.name) return enqueueBatchedPullRequestRefresh( repositoryID: repositoryID, repositoryRootURL: repositoryRootURL, worktrees: worktrees, - branches: branches + branches: branches, + detailBranches: detailBranches ) case .unknown: queuePullRequestRefresh( @@ -926,7 +929,8 @@ extension RepositoriesFeature { repositoryID: Repository.ID, repositoryRootURL: URL, worktrees: [Worktree], - branches: [String] + branches: [String], + detailBranches: [String] = [] ) -> Effect { let worktreeIDs = worktrees.map(\.id) let coordinatorClient = pullRequestRefreshCoordinator @@ -965,7 +969,8 @@ extension RepositoriesFeature { repositories: hostRemoteInfos, accountOverride: repositorySettings.githubAccountOverride, branches: branches, - worktreeIDs: worktreeIDs + worktreeIDs: worktreeIDs, + detailBranches: detailBranches ) ) } diff --git a/App/Sources/Features/Repositories/Views/PullRequestChecksPopoverView.swift b/App/Sources/Features/Repositories/Views/PullRequestChecksPopoverView.swift index 735bf67e..79cd3f9b 100644 --- a/App/Sources/Features/Repositories/Views/PullRequestChecksPopoverView.swift +++ b/App/Sources/Features/Repositories/Views/PullRequestChecksPopoverView.swift @@ -15,7 +15,7 @@ struct PullRequestChecksPopoverView: View { ) { self.pullRequest = pullRequest self.checks = checks - self.breakdown = PullRequestCheckBreakdown(checks: checks) + self.breakdown = pullRequest.statusCheckRollup?.counts ?? PullRequestCheckBreakdown(checks: checks) self.sortedChecks = checks.sorted { let left = Self.sortRank(for: $0.checkState) let right = Self.sortRank(for: $1.checkState) @@ -99,6 +99,13 @@ struct PullRequestChecksPopoverView: View { .font(.caption) } + if sortedChecks.isEmpty && breakdown.total > 0 { + // Background refreshes fetch check counts only; the selected worktree also fetches each check. + Text("Select this worktree to list each check.") + .font(.caption) + .foregroundStyle(.secondary) + } + if !sortedChecks.isEmpty { Divider() VStack(alignment: .leading) { diff --git a/App/Sources/Features/Repositories/Views/PullRequestStatusButton.swift b/App/Sources/Features/Repositories/Views/PullRequestStatusButton.swift index 04e757df..329f7cab 100644 --- a/App/Sources/Features/Repositories/Views/PullRequestStatusButton.swift +++ b/App/Sources/Features/Repositories/Views/PullRequestStatusButton.swift @@ -8,7 +8,7 @@ struct PullRequestStatusButton: View { var body: some View { PullRequestChecksPopoverButton(pullRequest: model.pullRequest) { - let breakdown = PullRequestCheckBreakdown(checks: model.statusChecks) + let breakdown = model.checkBreakdown let showsChecksRing = breakdown.total > 0 && model.state != "MERGED" HStack(spacing: 6) { PullRequestBadgeView( @@ -56,7 +56,7 @@ struct PullRequestStatusModel: Equatable { let number: Int let state: String? let title: String - let statusChecks: [GithubPullRequestStatusCheck] + let checkBreakdown: PullRequestCheckBreakdown let detailText: String? init?(pullRequest: GithubPullRequest?) { @@ -73,24 +73,23 @@ struct PullRequestStatusModel: Equatable { self.title = pullRequest.title if state == "MERGED" { self.detailText = nil - self.statusChecks = [] + self.checkBreakdown = PullRequestCheckBreakdown(checks: []) return } if state == "CLOSED" { self.detailText = nil - self.statusChecks = [] + self.checkBreakdown = PullRequestCheckBreakdown(checks: []) return } let isDraft = pullRequest.isDraft let prefix = isDraft ? "(Drafted) " : "" let mergeReadiness = PullRequestMergeReadiness(pullRequest: pullRequest) - let checks = pullRequest.statusCheckRollup?.checks ?? [] - self.statusChecks = checks + let breakdown = pullRequest.checkBreakdown + self.checkBreakdown = breakdown let checksDetail: String? - if checks.isEmpty { + if breakdown.total == 0 { checksDetail = nil } else { - let breakdown = PullRequestCheckBreakdown(checks: checks) let checksLabel = breakdown.total == 1 ? "check" : "checks" checksDetail = breakdown.summaryText + " \(checksLabel)" } diff --git a/App/Tests/BatchedPullRequestRefreshReducerTests.swift b/App/Tests/BatchedPullRequestRefreshReducerTests.swift index a3828dd8..954f9040 100644 --- a/App/Tests/BatchedPullRequestRefreshReducerTests.swift +++ b/App/Tests/BatchedPullRequestRefreshReducerTests.swift @@ -62,6 +62,37 @@ struct BatchedPullRequestRefreshReducerTests { #expect(request.branches == ["main", "feature"]) } + @Test func refreshListsChecksOnlyForTheSelectedWorktree() async { + let context = makeContext() + var initialState = context.state + initialState.selection = .worktree(context.featureWorktree.id) + let enqueued = LockIsolated<[PullRequestRefreshCoordinator.Request]>([]) + + let store = TestStore(initialState: initialState) { + RepositoriesFeature() + } withDependencies: { + $0.gitClient.githubRemoteInfos = { _ in [context.remoteInfo] } + $0.pullRequestRefreshCoordinator = PullRequestRefreshCoordinatorClient( + enqueue: { request in enqueued.withValue { $0.append(request) } }, + cancelHost: { _ in }, + reset: {} + ) + } + store.exhaustivity = .off + + await store.send( + .worktreeInfoEvent( + .repositoryPullRequestRefresh( + repositoryRootURL: context.repoRootURL, + worktreeIDs: context.worktreeIDs + ) + ) + ) + await store.finish() + + #expect(enqueued.value.map(\.detailBranches) == [[context.featureWorktree.name]]) + } + @Test func refreshWaitsForAllHostBatchesBeforeCompleting() async { let context = makeContext() let enqueued = LockIsolated<[PullRequestRefreshCoordinator.Request]>([]) diff --git a/App/Tests/PullRequestCheckCountsTests.swift b/App/Tests/PullRequestCheckCountsTests.swift new file mode 100644 index 00000000..04a5f822 --- /dev/null +++ b/App/Tests/PullRequestCheckCountsTests.swift @@ -0,0 +1,168 @@ +import Foundation +import Testing + +@testable import Prowl + +struct PullRequestCheckCountsTests { + @Test func countsOnlyRollupDecodesEveryState() throws { + let json = """ + { + "contexts": { + "checkRunCountsByState": [ + {"state": "SUCCESS", "count": 4}, + {"state": "NEUTRAL", "count": 1}, + {"state": "FAILURE", "count": 1}, + {"state": "TIMED_OUT", "count": 1}, + {"state": "ACTION_REQUIRED", "count": 1}, + {"state": "STARTUP_FAILURE", "count": 1}, + {"state": "STALE", "count": 1}, + {"state": "CANCELLED", "count": 2}, + {"state": "SKIPPED", "count": 1}, + {"state": "IN_PROGRESS", "count": 1}, + {"state": "QUEUED", "count": 1}, + {"state": "PENDING", "count": 1}, + {"state": "WAITING", "count": 1}, + {"state": "COMPLETED", "count": 1} + ], + "statusContextCountsByState": [ + {"state": "SUCCESS", "count": 2}, + {"state": "FAILURE", "count": 1}, + {"state": "ERROR", "count": 1}, + {"state": "EXPECTED", "count": 3}, + {"state": "PENDING", "count": 1} + ] + } + } + """ + let rollup = try JSONDecoder().decode(GithubPullRequestStatusCheckRollup.self, from: Data(json.utf8)) + + #expect(rollup.checks.isEmpty) + #expect( + rollup.breakdown + == PullRequestCheckBreakdown(passed: 7, failed: 7, inProgress: 6, expected: 3, skipped: 3) + ) + } + + // Each count state lands in the same bucket an individual check of that state lands in, so the + // badge reads the same whether a refresh fetched counts or the full list. + @Test( + arguments: [ + ("SUCCESS", GithubPullRequestStatusCheck(status: "COMPLETED", conclusion: "SUCCESS")), + ("NEUTRAL", GithubPullRequestStatusCheck(status: "COMPLETED", conclusion: "NEUTRAL")), + ("FAILURE", GithubPullRequestStatusCheck(status: "COMPLETED", conclusion: "FAILURE")), + ("TIMED_OUT", GithubPullRequestStatusCheck(status: "COMPLETED", conclusion: "TIMED_OUT")), + ("ACTION_REQUIRED", GithubPullRequestStatusCheck(status: "COMPLETED", conclusion: "ACTION_REQUIRED")), + ("STARTUP_FAILURE", GithubPullRequestStatusCheck(status: "COMPLETED", conclusion: "STARTUP_FAILURE")), + ("STALE", GithubPullRequestStatusCheck(status: "COMPLETED", conclusion: "STALE")), + ("CANCELLED", GithubPullRequestStatusCheck(status: "COMPLETED", conclusion: "CANCELLED")), + ("SKIPPED", GithubPullRequestStatusCheck(status: "COMPLETED", conclusion: "SKIPPED")), + ("IN_PROGRESS", GithubPullRequestStatusCheck(status: "IN_PROGRESS")), + ("QUEUED", GithubPullRequestStatusCheck(status: "QUEUED")), + ("PENDING", GithubPullRequestStatusCheck(status: "PENDING")), + ("WAITING", GithubPullRequestStatusCheck(status: "WAITING")), + ("COMPLETED", GithubPullRequestStatusCheck(status: "COMPLETED")), + ] + ) + func checkRunCountMatchesTheIndividualCheck(state: String, check: GithubPullRequestStatusCheck) throws { + let json = #"{"contexts":{"checkRunCountsByState":[{"state":"\#(state)","count":1}]}}"# + let rollup = try JSONDecoder().decode(GithubPullRequestStatusCheckRollup.self, from: Data(json.utf8)) + #expect(rollup.breakdown == PullRequestCheckBreakdown(checks: [check])) + } + + @Test( + arguments: [ + ("SUCCESS", GithubPullRequestStatusCheck(state: "SUCCESS")), + ("FAILURE", GithubPullRequestStatusCheck(state: "FAILURE")), + ("ERROR", GithubPullRequestStatusCheck(state: "ERROR")), + ("EXPECTED", GithubPullRequestStatusCheck(state: "EXPECTED")), + ("PENDING", GithubPullRequestStatusCheck(state: "PENDING")), + ] + ) + func statusContextCountMatchesTheIndividualContext(state: String, check: GithubPullRequestStatusCheck) throws { + let json = #"{"contexts":{"statusContextCountsByState":[{"state":"\#(state)","count":1}]}}"# + let rollup = try JSONDecoder().decode(GithubPullRequestStatusCheckRollup.self, from: Data(json.utf8)) + #expect(rollup.breakdown == PullRequestCheckBreakdown(checks: [check])) + } + + @Test func countsWinOverAListCappedAtOneHundred() throws { + let json = """ + { + "contexts": { + "checkRunCountsByState": [{"state": "SUCCESS", "count": 140}], + "nodes": [{"name": "build", "status": "COMPLETED", "conclusion": "SUCCESS"}] + } + } + """ + let rollup = try JSONDecoder().decode(GithubPullRequestStatusCheckRollup.self, from: Data(json.utf8)) + + #expect(rollup.checks.map(\.displayName) == ["build"]) + #expect(rollup.breakdown.passed == 140) + } + + @Test func mergeReadinessReadsCounts() { + let pullRequest = GithubPullRequest( + number: 1, + title: "PR", + state: "OPEN", + additions: 0, + deletions: 0, + isDraft: false, + reviewDecision: nil, + mergeable: "MERGEABLE", + mergeStateStatus: "UNSTABLE", + updatedAt: nil, + url: "https://example.com/pull/1", + headRefName: "feature", + baseRefName: "main", + commitsCount: 1, + authorLogin: "khoi", + statusCheckRollup: GithubPullRequestStatusCheckRollup( + checks: [], + counts: PullRequestCheckBreakdown(passed: 3, failed: 2, inProgress: 0, expected: 0, skipped: 0) + ) + ) + + #expect(PullRequestMergeReadiness(pullRequest: pullRequest).blockingReason == .checksFailed(2)) + } + + @Test func onlyDetailFieldsListEachCheck() { + let counts = pullRequestNodeFields(includeCheckDetails: false) + let details = pullRequestNodeFields(includeCheckDetails: true) + + #expect(counts.contains("checkRunCountsByState")) + #expect(counts.contains("statusContextCountsByState")) + #expect(!counts.contains("... on CheckRun")) + #expect(!counts.contains("first: 100")) + #expect(details.contains("checkRunCountsByState")) + #expect(details.contains("contexts(first: 100)")) + #expect(details.contains("... on CheckRun")) + } + + @Test func crossRepoQueryListsChecksOnlyForDetailBranches() async throws { + let probe = GithubBatchShellProbe() + let observedArguments = ObservedArguments() + let shell = makeBatchAcrossShellMock(probe: probe) { arguments in + await observedArguments.append(arguments) + return ShellOutput(stdout: crossRepoGraphQLResponse(for: arguments), stderr: "", exitCode: 0) + } + let client = GithubCLIClient.live(shell: shell) + let request = CrossRepoPullRequestRequest( + owner: "khoi", + repo: "repo", + branches: ["quiet", "selected", "other"], + detailBranches: ["selected"] + ) + + _ = try await client.batchPullRequestsAcrossRepositories("github.com", [request], nil) + + let arguments = try #require(await observedArguments.snapshot().first) + let query = try #require(arguments.first { $0.hasPrefix("query=") }) + #expect(query.components(separatedBy: "checkRunCountsByState").count - 1 == 3) + #expect(query.components(separatedBy: "... on CheckRun").count - 1 == 1) + let selectedBlock = try #require(query.range(of: "r0_b1:")) + let otherBlock = try #require(query.range(of: "r0_b2:")) + let detailRange = try #require(query.range(of: "... on CheckRun")) + #expect(detailRange.lowerBound > selectedBlock.lowerBound) + #expect(detailRange.lowerBound < otherBlock.lowerBound) + } +} diff --git a/App/Tests/PullRequestRefreshCoordinatorTests.swift b/App/Tests/PullRequestRefreshCoordinatorTests.swift index 52d9babc..bb961cec 100644 --- a/App/Tests/PullRequestRefreshCoordinatorTests.swift +++ b/App/Tests/PullRequestRefreshCoordinatorTests.swift @@ -183,6 +183,36 @@ struct PullRequestRefreshCoordinatorTests { #expect(failed == ["alpha"]) } + @Test func detailBranchesReachTheBatchedQuery() async throws { + let clock = TestClock() + let probe = CoordinatorProbe() + let outcomes = OutcomeCollector() + let coordinator = makeCoordinator( + probe: probe, clock: clock, outcomes: outcomes, + batched: { _, requests in successResult(for: requests) } + ) + + coordinator.enqueue(request(repo: "alpha", branches: ["feat-1", "feat-2"])) + coordinator.enqueue( + PullRequestRefreshCoordinator.Request( + repositoryID: "alpha", + repositoryRootURL: URL(fileURLWithPath: "/tmp/alpha"), + host: "github.com", + owner: "khoi", + repo: "alpha", + accountOverride: nil, + branches: ["feat-2"], + worktreeIDs: ["alpha-wt"], + detailBranches: ["feat-2"] + ) + ) + await advanceCoordinatorClock(clock, by: .milliseconds(250)) + await waitUntil { await probe.batchedCalls().count == 1 } + + let call = try #require(await probe.batchedCalls().first) + #expect(call.requests.map(\.detailBranches) == [["feat-2"]]) + } + @Test func inflightHostBuffersNewEnqueueAndFlushesAfterCompletion() async throws { let clock = TestClock() let probe = CoordinatorProbe() diff --git a/docs-ai/028-pr-status-tracking/000-plan.md b/docs-ai/028-pr-status-tracking/000-plan.md index 49a46b47..c19c44a3 100644 --- a/docs-ai/028-pr-status-tracking/000-plan.md +++ b/docs-ai/028-pr-status-tracking/000-plan.md @@ -99,3 +99,5 @@ As of the anchor wave (reconstructed from #256/#305/#366/#379 PR descriptions): [003-status-fidelity-and-refresh-cadence.md](003-status-fidelity-and-refresh-cadence.md) - Updated 2026-07-08: flicker elimination and explicit no-PR semantics (#533 → #538 → #539) — see [004-flicker-and-no-pr-semantics.md](004-flicker-and-no-pr-semantics.md) +- Updated 2026-10-02: badge refreshes fetch per-state check counts, and only the selected + worktree's pull request lists each check — see [007-check-counts.md](007-check-counts.md) diff --git a/docs-ai/028-pr-status-tracking/007-check-counts.md b/docs-ai/028-pr-status-tracking/007-check-counts.md new file mode 100644 index 00000000..ee1b69f7 --- /dev/null +++ b/docs-ai/028-pr-status-tracking/007-check-counts.md @@ -0,0 +1,49 @@ +# 028.007 — Check Counts for Badges + +## Context + +PR polling tripped GitHub's GraphQL secondary rate limit on a shared account twice +(2026-09-29 and 2026-09-30) while the account's hourly point budget stayed almost full: +4,979 of 5,000 remained during the second incident. The batch query was cheap in points +but expensive for GitHub to resolve. Every branch asked for up to five pull requests, +and every pull request for `statusCheckRollup.contexts(first: 100)` with each check's +name, status, conclusion, timestamps, and URL. A 34-branch query could ask GitHub to +resolve up to 17,000 check nodes, which secondary limits on server time count +against the account. + +Only two places use the individual checks: the checks popover and the "Copy failing job +URL" and "Open failing check" actions. The badge, the checks ring, the toolbar status +text, merge readiness, and the command palette's failing-check items need per-state +counts. + +## Change + +- **Counts on every pull request.** The query selects + `contexts { checkRunCountsByState { state count } statusContextCountsByState { state count } }` + for every pull request, which needs no pagination arguments and adds no nodes. + `GithubPullRequestStatusCheckRollup` decodes them into `counts`, mapping each state onto + the bucket `GithubPullRequestStatusCheck.checkState` assigns to a single check of that + state, and exposes `breakdown`: the counts when present, else a breakdown of the list. + `GithubPullRequest.checkBreakdown` is the reader the badge, ring, status text, merge + readiness, and palette use. +- **List for the selected worktree only.** `CrossRepoPullRequestRequest.detailBranches` + names branches whose pull request also lists each check with + `contexts(first: 100) { nodes { ... } }`. The reducer fills it with the selected + worktree's branch, and the coordinator carries it through merged requests. Selecting a + worktree already triggers an immediate refresh of its repository, so the list arrives + with it. Counts win over the list when both are present, since the list stops at 100. +- **Shared selection.** Both query builders take the pull request fields from + `pullRequestNodeFields(includeCheckDetails:)` instead of two copies. +- **Popover.** For a pull request fetched with counts only, the popover shows the ring + and summary and the line "Select this worktree to list each check." + +The single-repository fallback query asks for counts only. + +## Refs + +Tests: `App/Tests/PullRequestCheckCountsTests.swift` (every count state decodes into +the bucket of the matching single check; counts win over a capped list; merge readiness +reads counts; only detail branches list checks in the batch query), +`PullRequestRefreshCoordinatorTests.swift` (detail branches reach the batched query), +`BatchedPullRequestRefreshReducerTests.swift` (only the selected worktree's branch is a +detail branch). diff --git a/docs/components/github-pull-requests.md b/docs/components/github-pull-requests.md index fdf0759f..436a00c3 100644 --- a/docs/components/github-pull-requests.md +++ b/docs/components/github-pull-requests.md @@ -33,6 +33,9 @@ GitHub remote, stale PR badges are cleared. - Review decision (approved / changes requested / pending). - **CI status:** a rollup of all checks (success / failure / in-progress / expected / skipped) with failing/success counts and per-check detail URLs. + Every worktree's pull request carries the counts; the selected worktree's + also lists each check with its URL. Hovering the PR tag of another worktree + shows the counts and asks you to select that worktree to list each check. - **Merge readiness:** Prowl evaluates blockers in order — merge conflicts, changes requested, failed checks, other non-mergeable states. - **Merge queue:** for repos that use GitHub merge queues, an open PR waiting in -- 2.54.0 From c37ea9d53af74f50013e57794cc8626f34160129 Mon Sep 17 00:00:00 2001 From: Simon Heimlicher Date: Fri, 2 Oct 2026 01:47:39 +0200 Subject: [PATCH 2/4] Keep the selected worktree's checks when a batch falls back When the cross-repository query failed or timed out, the per-repository fallback asked for counts only, so the selected worktree lost its check list until a later refresh, and the failing-check actions had no URL to act on. The fallback query now takes the same detail branches. --- .../Clients/Github/GithubCLIClient.swift | 21 ++++++---- .../Clients/Github/GithubCLIModels.swift | 2 + .../PullRequestRefreshCoordinator.swift | 1 + ...epositoriesFeature+WorkspaceChildren.swift | 1 + ...atchedPullRequestRefreshReducerTests.swift | 2 +- App/Tests/GithubCLIClientTests.swift | 6 +-- App/Tests/PullRequestCheckCountsTests.swift | 30 ++++++++++++++ .../PullRequestRefreshCoordinatorTests.swift | 41 +++++++++++++++++-- App/Tests/RepositoriesFeatureTests.swift | 4 +- .../007-check-counts.md | 8 ++-- 10 files changed, 95 insertions(+), 21 deletions(-) diff --git a/App/Sources/Clients/Github/GithubCLIClient.swift b/App/Sources/Clients/Github/GithubCLIClient.swift index f6aa0b7d..25ab3d10 100644 --- a/App/Sources/Clients/Github/GithubCLIClient.swift +++ b/App/Sources/Clients/Github/GithubCLIClient.swift @@ -180,7 +180,8 @@ struct GithubCLIClient: Sendable { var resolveRemoteInfo: @Sendable (URL) async -> GithubRemoteInfo? var latestRun: @Sendable (URL, String, GithubAccountOverride?) async throws -> GithubWorkflowRun? var batchPullRequests: - @Sendable (String, String, String, [String], GithubAccountOverride?) async throws -> [String: GithubPullRequest] + @Sendable (String, String, String, [String], Set, GithubAccountOverride?) async throws -> [String: + GithubPullRequest] var batchPullRequestsAcrossRepositories: @Sendable (String, [CrossRepoPullRequestRequest], GithubAccountOverride?) async throws -> CrossRepoPullRequestResult var mergePullRequest: @@ -225,7 +226,7 @@ extension GithubCLIClient: DependencyKey { defaultBranch: { _ in "main" }, resolveRemoteInfo: { _ in nil }, latestRun: { _, _, _ in nil }, - batchPullRequests: { _, _, _, _, _ in [:] }, + batchPullRequests: { _, _, _, _, _, _ in [:] }, batchPullRequestsAcrossRepositories: { _, _, _ in CrossRepoPullRequestResult() }, mergePullRequest: { _, _, _, _, _ in }, closePullRequest: { _, _, _, _ in }, @@ -336,8 +337,11 @@ nonisolated private func latestRunFetcher( nonisolated private func batchPullRequestsFetcher( shell: ShellClient, resolver: GithubCLIExecutableResolver -) -> @Sendable (String, String, String, [String], GithubAccountOverride?) async throws -> [String: GithubPullRequest] { - { host, owner, repo, branches, accountOverride in +) + -> @Sendable (String, String, String, [String], Set, GithubAccountOverride?) async throws -> [String: + GithubPullRequest] +{ + { host, owner, repo, branches, detailBranches, accountOverride in try await withExpectedGithubAccount( shell: shell, resolver: resolver, @@ -348,7 +352,7 @@ nonisolated private func batchPullRequestsFetcher( guard !dedupedBranches.isEmpty else { return [:] } - let request = GithubPullRequestsRequest(host: host, owner: owner, repo: repo) + let request = GithubPullRequestsRequest(host: host, owner: owner, repo: repo, detailBranches: detailBranches) let chunks = makeBranchChunks( dedupedBranches, chunkSize: batchPullRequestsChunkSize @@ -1152,7 +1156,7 @@ nonisolated private func fetchPullRequestsChunk( chunk: [String], chunkIndex: Int ) async throws -> (Int, [String: GithubPullRequest]) { - let (query, aliasMap) = makeBatchPullRequestsQuery(branches: chunk) + let (query, aliasMap) = makeBatchPullRequestsQuery(branches: chunk, detailBranches: request.detailBranches) let output = try await runGh( shell: shell, resolver: resolver, @@ -1186,7 +1190,8 @@ nonisolated private func fetchPullRequestsChunk( } nonisolated private func makeBatchPullRequestsQuery( - branches: [String] + branches: [String], + detailBranches: Set ) -> (query: String, aliasMap: [String: String]) { var aliasMap: [String: String] = [:] var selections: [String] = [] @@ -1198,7 +1203,7 @@ nonisolated private func makeBatchPullRequestsQuery( let selection = """ \(alias): pullRequests(first: 5, states: [OPEN, MERGED, CLOSED], headRefName: \"\(escapedBranch)\", \(orderBy)) { nodes { - \(pullRequestNodeFields(includeCheckDetails: false)) + \(pullRequestNodeFields(includeCheckDetails: detailBranches.contains(branch))) } } """ diff --git a/App/Sources/Clients/Github/GithubCLIModels.swift b/App/Sources/Clients/Github/GithubCLIModels.swift index 0986df5f..fdc0f6a3 100644 --- a/App/Sources/Clients/Github/GithubCLIModels.swift +++ b/App/Sources/Clients/Github/GithubCLIModels.swift @@ -187,6 +187,8 @@ struct GithubPullRequestsRequest: Sendable { let host: String let owner: String let repo: String + /// Branches whose pull request also lists each check, not only the per-state counts. + var detailBranches: Set = [] } nonisolated struct GithubRepoViewRemoteInfoResponse: Decodable, Sendable { diff --git a/App/Sources/Features/Repositories/BusinessLogic/PullRequestRefreshCoordinator.swift b/App/Sources/Features/Repositories/BusinessLogic/PullRequestRefreshCoordinator.swift index ce8ce728..9c3d9843 100644 --- a/App/Sources/Features/Repositories/BusinessLogic/PullRequestRefreshCoordinator.swift +++ b/App/Sources/Features/Repositories/BusinessLogic/PullRequestRefreshCoordinator.swift @@ -285,6 +285,7 @@ final class PullRequestRefreshCoordinator { repoGroup.key.owner, repoGroup.key.repo, repoGroup.branches, + repoGroup.detailBranches, key.accountOverride ) return .success(repoGroup.key, prs) diff --git a/App/Sources/Features/Repositories/Reducer/RepositoriesFeature+WorkspaceChildren.swift b/App/Sources/Features/Repositories/Reducer/RepositoriesFeature+WorkspaceChildren.swift index 4b59faf1..5e10e84b 100644 --- a/App/Sources/Features/Repositories/Reducer/RepositoriesFeature+WorkspaceChildren.swift +++ b/App/Sources/Features/Repositories/Reducer/RepositoriesFeature+WorkspaceChildren.swift @@ -94,6 +94,7 @@ extension RepositoriesFeature { remote.owner, remote.repo, [branch], + [], nil ) return pullRequestsByBranch?[branch] diff --git a/App/Tests/BatchedPullRequestRefreshReducerTests.swift b/App/Tests/BatchedPullRequestRefreshReducerTests.swift index 954f9040..23260e64 100644 --- a/App/Tests/BatchedPullRequestRefreshReducerTests.swift +++ b/App/Tests/BatchedPullRequestRefreshReducerTests.swift @@ -21,7 +21,7 @@ struct BatchedPullRequestRefreshReducerTests { Issue.record("gh resolveRemoteInfo should not run when git remotes resolve") return nil } - $0.githubCLI.batchPullRequests = { _, _, _, _, _ in + $0.githubCLI.batchPullRequests = { _, _, _, _, _, _ in Issue.record("Legacy batchPullRequests should not run on coordinator path") return [:] } diff --git a/App/Tests/GithubCLIClientTests.swift b/App/Tests/GithubCLIClientTests.swift index 3ac25c9f..49e0c931 100644 --- a/App/Tests/GithubCLIClientTests.swift +++ b/App/Tests/GithubCLIClientTests.swift @@ -246,7 +246,7 @@ struct GithubCLIClientTests { let client = GithubCLIClient.live(shell: shell) let branches = (0..<100).map { "feature-\($0)" } - _ = try await client.batchPullRequests("github.com", "khoi", "repo", branches, nil) + _ = try await client.batchPullRequests("github.com", "khoi", "repo", branches, [], nil) let snapshot = await probe.snapshot() #expect(snapshot.ghCallCount == 4) @@ -296,7 +296,7 @@ struct GithubCLIClientTests { let branches = (0..<30).map { "feature-\($0)" } do { - _ = try await client.batchPullRequests("github.com", "khoi", "repo", branches, nil) + _ = try await client.batchPullRequests("github.com", "khoi", "repo", branches, [], nil) Issue.record("Expected batchPullRequests to throw") } catch let error as GithubCLIError { switch error { @@ -336,7 +336,7 @@ struct GithubCLIClientTests { let uniqueBranches = (0..<30).map { "feature-\($0)" } let branches = uniqueBranches + ["feature-0", "feature-1", "feature-2", "", ""] - let result = try await client.batchPullRequests("github.com", "khoi", "repo", branches, nil) + let result = try await client.batchPullRequests("github.com", "khoi", "repo", branches, [], nil) #expect(result.isEmpty) let snapshot = await probe.snapshot() diff --git a/App/Tests/PullRequestCheckCountsTests.swift b/App/Tests/PullRequestCheckCountsTests.swift index 04a5f822..8653cdf7 100644 --- a/App/Tests/PullRequestCheckCountsTests.swift +++ b/App/Tests/PullRequestCheckCountsTests.swift @@ -165,4 +165,34 @@ struct PullRequestCheckCountsTests { #expect(detailRange.lowerBound > selectedBlock.lowerBound) #expect(detailRange.lowerBound < otherBlock.lowerBound) } + + @Test func singleRepositoryQueryListsChecksOnlyForDetailBranches() async throws { + let observedArguments = ObservedArguments() + let shell = ShellClient( + run: { executableURL, _, _ in + if executableURL.lastPathComponent == "which" { + return ShellOutput(stdout: "/usr/bin/gh", stderr: "", exitCode: 0) + } + return ShellOutput(stdout: "", stderr: "", exitCode: 0) + }, + runLoginImpl: { executableURL, arguments, _, _ in + guard executableURL.lastPathComponent == "gh" else { + return ShellOutput(stdout: "", stderr: "", exitCode: 0) + } + await observedArguments.append(arguments) + return ShellOutput(stdout: #"{"data":{"repository":{}}}"#, stderr: "", exitCode: 0) + } + ) + let client = GithubCLIClient.live(shell: shell) + + _ = try await client.batchPullRequests("github.com", "khoi", "repo", ["quiet", "selected"], ["selected"], nil) + + let arguments = try #require(await observedArguments.snapshot().first) + let query = try #require(arguments.first { $0.hasPrefix("query=") }) + #expect(query.components(separatedBy: "checkRunCountsByState").count - 1 == 2) + #expect(query.components(separatedBy: "... on CheckRun").count - 1 == 1) + let detailRange = try #require(query.range(of: "... on CheckRun")) + let selectedBlock = try #require(query.range(of: "branch1:")) + #expect(detailRange.lowerBound > selectedBlock.lowerBound) + } } diff --git a/App/Tests/PullRequestRefreshCoordinatorTests.swift b/App/Tests/PullRequestRefreshCoordinatorTests.swift index bb961cec..b17a5e04 100644 --- a/App/Tests/PullRequestRefreshCoordinatorTests.swift +++ b/App/Tests/PullRequestRefreshCoordinatorTests.swift @@ -213,6 +213,38 @@ struct PullRequestRefreshCoordinatorTests { #expect(call.requests.map(\.detailBranches) == [["feat-2"]]) } + @Test func fallbackKeepsDetailBranches() async throws { + let clock = TestClock() + let probe = CoordinatorProbe() + let outcomes = OutcomeCollector() + let coordinator = makeCoordinator( + probe: probe, + clock: clock, + outcomes: outcomes, + batched: { _, _ in + throw GithubCLIError.commandFailed("network down") + } + ) + + coordinator.enqueue( + PullRequestRefreshCoordinator.Request( + repositoryID: "alpha", + repositoryRootURL: URL(fileURLWithPath: "/tmp/alpha"), + host: "github.com", + owner: "khoi", + repo: "alpha", + accountOverride: nil, + branches: ["feat-1", "feat-2"], + worktreeIDs: ["alpha-wt"], + detailBranches: ["feat-2"] + ) + ) + await advanceCoordinatorClock(clock, by: .milliseconds(250)) + await waitUntil { await probe.legacyCalls().count == 1 } + + #expect(await probe.legacyCalls().map(\.detailBranches) == [["feat-2"]]) + } + @Test func inflightHostBuffersNewEnqueueAndFlushesAfterCompletion() async throws { let clock = TestClock() let probe = CoordinatorProbe() @@ -674,8 +706,8 @@ private func makeCoordinator( await probe.recordBatched(host: host, requests: requests, accountOverride: accountOverride) return try await batched(host, requests) } - client.batchPullRequests = { host, owner, repo, branches, _ in - await probe.recordLegacy(host: host, owner: owner, repo: repo, branches: branches) + client.batchPullRequests = { host, owner, repo, branches, detailBranches, _ in + await probe.recordLegacy(host: host, owner: owner, repo: repo, branches: branches, detailBranches: detailBranches) return try await legacy(host, owner, repo, branches) } return PullRequestRefreshCoordinator( @@ -776,6 +808,7 @@ actor CoordinatorProbe { let owner: String let repo: String let branches: [String] + let detailBranches: Set } private var batched: [BatchedCall] = [] @@ -789,8 +822,8 @@ actor CoordinatorProbe { batched.append(BatchedCall(host: host, requests: requests, accountOverride: accountOverride)) } - func recordLegacy(host: String, owner: String, repo: String, branches: [String]) { - legacy.append(LegacyCall(host: host, owner: owner, repo: repo, branches: branches)) + func recordLegacy(host: String, owner: String, repo: String, branches: [String], detailBranches: Set) { + legacy.append(LegacyCall(host: host, owner: owner, repo: repo, branches: branches, detailBranches: detailBranches)) } func batchedCalls() -> [BatchedCall] { diff --git a/App/Tests/RepositoriesFeatureTests.swift b/App/Tests/RepositoriesFeatureTests.swift index ce258367..e997b840 100644 --- a/App/Tests/RepositoriesFeatureTests.swift +++ b/App/Tests/RepositoriesFeatureTests.swift @@ -5806,7 +5806,7 @@ struct RepositoriesFeatureTests { Issue.record("remoteInfo should not be requested when GitHub integration is unavailable") return nil } - $0.githubCLI.batchPullRequests = { _, _, _, _, _ in + $0.githubCLI.batchPullRequests = { _, _, _, _, _, _ in Issue.record("batchPullRequests should not run when GitHub integration is unavailable") return [:] } @@ -7441,7 +7441,7 @@ struct RepositoriesFeatureTests { #expect(url.path(percentEncoded: false) == childID) return GithubRemoteInfo(host: "github.com", owner: "onevcat", repo: "app") } - $0.githubCLI.batchPullRequests = { host, owner, repo, branches, accountOverride in + $0.githubCLI.batchPullRequests = { host, owner, repo, branches, _, accountOverride in #expect(accountOverride == nil) calls.withValue { $0.append( diff --git a/docs-ai/028-pr-status-tracking/007-check-counts.md b/docs-ai/028-pr-status-tracking/007-check-counts.md index ee1b69f7..7bc0fb7c 100644 --- a/docs-ai/028-pr-status-tracking/007-check-counts.md +++ b/docs-ai/028-pr-status-tracking/007-check-counts.md @@ -37,13 +37,15 @@ counts. - **Popover.** For a pull request fetched with counts only, the popover shows the ring and summary and the line "Select this worktree to list each check." -The single-repository fallback query asks for counts only. +The single-repository fallback query takes the same detail branches, so a failed batch does +not drop the selected worktree's checks. ## Refs Tests: `App/Tests/PullRequestCheckCountsTests.swift` (every count state decodes into the bucket of the matching single check; counts win over a capped list; merge readiness -reads counts; only detail branches list checks in the batch query), -`PullRequestRefreshCoordinatorTests.swift` (detail branches reach the batched query), +reads counts; only detail branches list checks in the batch and single-repository queries), +`PullRequestRefreshCoordinatorTests.swift` (detail branches reach the batched query and the +fallback), `BatchedPullRequestRefreshReducerTests.swift` (only the selected worktree's branch is a detail branch). -- 2.54.0 From 1f1cbd32a532a8d90d70b3712e25773dd74c6dfe Mon Sep 17 00:00:00 2001 From: Simon Heimlicher Date: Sat, 3 Oct 2026 16:38:23 +0200 Subject: [PATCH 3/4] Record check counts in the PR status action log --- docs-ai/028-pr-status-tracking/001-action.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/docs-ai/028-pr-status-tracking/001-action.md b/docs-ai/028-pr-status-tracking/001-action.md index 3c2e2ab0..c1a3b556 100644 --- a/docs-ai/028-pr-status-tracking/001-action.md +++ b/docs-ai/028-pr-status-tracking/001-action.md @@ -42,6 +42,12 @@ Grouped by problem wave; rows chronological within each wave. | 2026-07-05 | Supersedes #533: fix the nil-literal no-op clear (`updateValue(nil, forKey:)`); order-independent cross-host suppression via `prRefreshFailedBatchRepositoryIDs`; the missing non-empty-set test coverage | PR #538 → [004](004-flicker-and-no-pr-semantics.md) | | 2026-07-08 | Preserve last-known `mergeable`/`mergeStateStatus` when GitHub returns transient `UNKNOWN` (kills the "Blocked" flash) | PR #539 → [004](004-flicker-and-no-pr-semantics.md) | +### Wave 5 — Rate limits & refresh load (October) + +| Date | Change | Ref | +| --- | --- | --- | +| 2026-10-02 | Badges read per-state check counts (`checkRunCountsByState`, `statusContextCountsByState`); only the selected worktree's pull request lists each check | [007](007-check-counts.md) | + ## Outcome & current state (as of 2026-07-12) Client layer, `supacode/Clients/Github/`: -- 2.54.0 From a458cd0839ba7d90a34359821454da48ae78c644 Mon Sep 17 00:00:00 2001 From: onevcat Date: Fri, 9 Oct 2026 09:28:05 +0900 Subject: [PATCH 4/4] Read the check counts in the refresh cadence A background refresh now carries each pull request's checks as per-state counts without the list, so the cadence reads GithubPullRequest.checkBreakdown. Reading the list would see no running checks and move a pull request with checks in progress to the settled 3-minute cadence. Also pass the new detail-branch argument at the two batchPullRequests call sites that the rate-limit gate tests added. --- .../PullRequestRefreshCadence.swift | 5 ++-- App/Tests/GithubCLIClientTests.swift | 2 +- App/Tests/GithubRateLimitTests.swift | 2 +- .../PullRequestRefreshCadenceTests.swift | 29 ++++++++++++++++++- .../008-cadence-by-state.md | 7 +++-- 5 files changed, 37 insertions(+), 8 deletions(-) diff --git a/App/Sources/Features/Repositories/BusinessLogic/PullRequestRefreshCadence.swift b/App/Sources/Features/Repositories/BusinessLogic/PullRequestRefreshCadence.swift index 2a0aaa86..97180c36 100644 --- a/App/Sources/Features/Repositories/BusinessLogic/PullRequestRefreshCadence.swift +++ b/App/Sources/Features/Repositories/BusinessLogic/PullRequestRefreshCadence.swift @@ -60,9 +60,10 @@ nonisolated enum PullRequestRefreshCadence { } } - // Checks still running, GitHub still computing mergeability, or a place in the merge queue. + // Checks still running, GitHub still computing mergeability, or a place in the merge queue. A background + // refresh carries only the per-state check counts, so the breakdown is read, never the check list. private static func isActive(_ pullRequest: GithubPullRequest) -> Bool { - let checks = PullRequestCheckBreakdown(checks: pullRequest.statusCheckRollup?.checks ?? []) + let checks = pullRequest.checkBreakdown if checks.inProgress + checks.expected > 0 { return true } diff --git a/App/Tests/GithubCLIClientTests.swift b/App/Tests/GithubCLIClientTests.swift index 921f8e5f..e092d5ef 100644 --- a/App/Tests/GithubCLIClientTests.swift +++ b/App/Tests/GithubCLIClientTests.swift @@ -633,7 +633,7 @@ struct GithubCLIClientTests { message: "Could not resolve to a Repository with the name 'khoi/gone'." ) ) { - _ = try await client.batchPullRequests("github.com", "khoi", "gone", ["feature"], nil) + _ = try await client.batchPullRequests("github.com", "khoi", "gone", ["feature"], [], nil) } } diff --git a/App/Tests/GithubRateLimitTests.swift b/App/Tests/GithubRateLimitTests.swift index f9faaa93..f929d863 100644 --- a/App/Tests/GithubRateLimitTests.swift +++ b/App/Tests/GithubRateLimitTests.swift @@ -334,7 +334,7 @@ struct GithubRateLimitedClientTests { _ = try await client.batchPullRequestsAcrossRepositories("github.com", [request], nil) } await #expect(throws: GithubCLIError.rateLimited(retryAt: retryAt)) { - _ = try await client.batchPullRequests("github.com", "octo", "repo", ["feature"], nil) + _ = try await client.batchPullRequests("github.com", "octo", "repo", ["feature"], [], nil) } await #expect(throws: GithubCLIError.rateLimited(retryAt: retryAt)) { try await client.mergePullRequest( diff --git a/App/Tests/PullRequestRefreshCadenceTests.swift b/App/Tests/PullRequestRefreshCadenceTests.swift index ab6e9b9d..88a2e82a 100644 --- a/App/Tests/PullRequestRefreshCadenceTests.swift +++ b/App/Tests/PullRequestRefreshCadenceTests.swift @@ -35,6 +35,31 @@ struct PullRequestRefreshCadenceTests { secondsSinceAnswer: 1, isDue: true ), + // A background refresh reports the checks as per-state counts, without the list. + Case( + pullRequest: pullRequest( + state: "OPEN", + counts: PullRequestCheckBreakdown(passed: 2, failed: 0, inProgress: 1, expected: 0, skipped: 0) + ), + secondsSinceAnswer: 1, + isDue: true + ), + Case( + pullRequest: pullRequest( + state: "OPEN", + counts: PullRequestCheckBreakdown(passed: 0, failed: 0, inProgress: 0, expected: 1, skipped: 0) + ), + secondsSinceAnswer: 1, + isDue: true + ), + Case( + pullRequest: pullRequest( + state: "OPEN", + counts: PullRequestCheckBreakdown(passed: 3, failed: 1, inProgress: 0, expected: 0, skipped: 1) + ), + secondsSinceAnswer: 179, + isDue: false + ), Case(pullRequest: pullRequest(state: "OPEN", mergeable: "UNKNOWN"), secondsSinceAnswer: 1, isDue: true), Case(pullRequest: pullRequest(state: "OPEN", mergeStateStatus: "UNKNOWN"), secondsSinceAnswer: 1, isDue: true), Case(pullRequest: pullRequest(state: "OPEN", queued: true), secondsSinceAnswer: 1, isDue: true), @@ -108,6 +133,7 @@ nonisolated private func checkpoint(for pullRequest: GithubPullRequest?) -> Pull nonisolated private func pullRequest( state: String, checks: [GithubPullRequestStatusCheck] = [], + counts: PullRequestCheckBreakdown? = nil, mergeable: String? = "MERGEABLE", mergeStateStatus: String? = "CLEAN", queued: Bool = false @@ -128,7 +154,8 @@ nonisolated private func pullRequest( baseRefName: "main", commitsCount: 1, authorLogin: "khoi", - statusCheckRollup: checks.isEmpty ? nil : GithubPullRequestStatusCheckRollup(checks: checks), + statusCheckRollup: checks.isEmpty && counts == nil + ? nil : GithubPullRequestStatusCheckRollup(checks: checks, counts: counts), mergeQueueEntry: queued ? GithubMergeQueueEntry(position: 1, estimatedTimeToMerge: nil, state: "QUEUED") : nil ) } diff --git a/docs-ai/028-pr-status-tracking/008-cadence-by-state.md b/docs-ai/028-pr-status-tracking/008-cadence-by-state.md index c422ca61..c0b2044b 100644 --- a/docs-ai/028-pr-status-tracking/008-cadence-by-state.md +++ b/docs-ai/028-pr-status-tracking/008-cadence-by-state.md @@ -59,9 +59,10 @@ The HEAD watcher sees branch switches, not pushes or new commits, so a pull requ opened outside Prowl appears within the no-pull-request interval, or immediately when its worktree is selected. -`isActive` reads checks from `statusCheckRollup.checks`. Combined with the check-count -change (amendment 007, a separate change), it reads `GithubPullRequest.checkBreakdown` -instead, since background refreshes then carry counts without a list. +`isActive` reads the checks through `GithubPullRequest.checkBreakdown`, not from +`statusCheckRollup.checks`: since the check-count change (amendment 007), a background +refresh carries only the per-state counts, and a pull request whose checks are still +running must stay on the every-refresh cadence while its list is absent. ## Refs -- 2.54.0