Skip to content

Commit 7f6362e

Browse files
authored
Don't fetch Padawan PRs again just for the Chat Sessions view (#7652)
instead use the data we already fetched. Also, only run one refresh of Padawan PR status at a time
1 parent 13a4c59 commit 7f6362e

3 files changed

Lines changed: 114 additions & 57 deletions

File tree

src/github/common.ts

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import * as OctokitRest from '@octokit/rest';
66
import { Endpoints } from '@octokit/types';
77
import { ChatSessionStatus, Uri } from 'vscode';
88
import { Repository } from '../api/api';
9+
import { CopilotPRStatus } from '../common/copilot';
910
import { GitHubRemote } from '../common/remote';
1011
import { EventType, TimelineEvent } from '../common/timelineEvent';
1112
import { SessionInfo, SessionSetupStep } from './copilotApi';
@@ -175,3 +176,16 @@ export function copilotEventToSessionStatus(event: TimelineEvent | undefined): C
175176
return ChatSessionStatus.InProgress;
176177
}
177178
}
179+
180+
export function copilotPRStatusToSessionStatus(event: CopilotPRStatus): ChatSessionStatus {
181+
switch (event) {
182+
case CopilotPRStatus.Started:
183+
return ChatSessionStatus.InProgress;
184+
case CopilotPRStatus.Completed:
185+
return ChatSessionStatus.Completed;
186+
case CopilotPRStatus.Failed:
187+
return ChatSessionStatus.Failed;
188+
default:
189+
return ChatSessionStatus.InProgress;
190+
}
191+
}

src/github/copilotPrWatcher.ts

Lines changed: 86 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,10 @@ export class CopilotStateModel extends Disposable {
121121
error: errorCount
122122
};
123123
}
124+
125+
get all(): { item: PullRequestModel, status: CopilotPRStatus }[] {
126+
return Array.from(this._states.values());
127+
}
124128
}
125129

126130
export class CopilotPRWatcher extends Disposable {
@@ -140,10 +144,19 @@ export class CopilotPRWatcher extends Disposable {
140144
private _initialize() {
141145
this._getStateChanges();
142146
this._pollForChanges();
143-
const updateState = debounce(() => this._getStateChanges(), 50);
147+
const updateFullState = debounce(() => this._getStateChanges(), 50);
144148
this._register(this._reposManager.onDidChangeAnyPullRequests(e => {
145149
if (e.some(pr => COPILOT_ACCOUNTS[pr.model.author.login])) {
146-
updateState();
150+
if (e.some(pr => this._model.get(pr.model.remote.owner, pr.model.remote.repositoryName, pr.model.number) === CopilotPRStatus.None)) {
151+
// A PR we don't know about was updated
152+
updateFullState();
153+
} else {
154+
for (const pr of e) {
155+
if (pr.model instanceof PullRequestModel) {
156+
this._updateSingleState(pr.model);
157+
}
158+
}
159+
}
147160
}
148161
}));
149162
this._register(PullRequestOverviewPanel.onVisible(e => this._model.clearNotification(e.remote.owner, e.remote.repositoryName, e.number)));
@@ -180,58 +193,85 @@ export class CopilotPRWatcher extends Disposable {
180193
return this._currentUser;
181194
}
182195

196+
private async _updateSingleState(pr: PullRequestModel): Promise<void> {
197+
const changes: { pullRequestModel: PullRequestModel, status: CopilotPRStatus }[] = [];
198+
199+
const copilotEvents = await pr.getCopilotTimelineEvents(pr);
200+
let latestEvent = copilotEventToStatus(copilotEvents[copilotEvents.length - 1]);
201+
if (latestEvent === CopilotPRStatus.None) {
202+
if (!COPILOT_ACCOUNTS[pr.author.login]) {
203+
return;
204+
}
205+
latestEvent = CopilotPRStatus.Started;
206+
}
207+
const lastStatus = this._model.get(pr.remote.owner, pr.remote.repositoryName, pr.number) ?? CopilotPRStatus.None;
208+
if (latestEvent !== lastStatus) {
209+
changes.push({ pullRequestModel: pr, status: latestEvent });
210+
}
211+
this._model.set(changes);
212+
}
213+
214+
private _getStateChangesPromise: Promise<boolean> | undefined;
183215
private async _getStateChanges(): Promise<boolean> {
184-
const query = this._queriesIncludeCopilot();
185-
if (!query) {
186-
return false;
216+
// Return the existing in-flight promise if one exists
217+
if (this._getStateChangesPromise) {
218+
return this._getStateChangesPromise;
187219
}
188-
const stateChanges: { owner: string; repo: string; prNumber: number; status: CopilotPRStatus }[] = [];
189-
const unseenKeys: Set<string> = new Set(this._model.keys());
190-
let initialized = 0;
191220

192-
const changes: { pullRequestModel: PullRequestModel, status: CopilotPRStatus }[] = [];
193-
for (const folderManager of this._reposManager.folderManagers) {
194-
// It doesn't matter which repo we use since the query will specify the owner/repo.
195-
const githubRepository = folderManager.gitHubRepositories[0];
196-
if (!githubRepository) {
197-
continue;
198-
}
199-
initialized++;
200-
const prs = await folderManager.getPullRequestsForCategory(githubRepository, await variableSubstitution(query, undefined, await folderManager.getPullRequestDefaults(), await this._getCurrentUser(folderManager)));
201-
for (const pr of prs?.items ?? []) {
202-
unseenKeys.delete(this._model.makeKey(pr.remote.owner, pr.remote.repositoryName, pr.number));
203-
const copilotEvents = await pr.getCopilotTimelineEvents(pr);
204-
let latestEvent = copilotEventToStatus(copilotEvents[copilotEvents.length - 1]);
205-
if (latestEvent === CopilotPRStatus.None) {
206-
if (!COPILOT_ACCOUNTS[pr.author.login]) {
221+
// Create and store the in-flight promise, and ensure it's cleared when done
222+
this._getStateChangesPromise = (async () => {
223+
try {
224+
const query = this._queriesIncludeCopilot();
225+
if (!query) {
226+
return false;
227+
}
228+
const unseenKeys: Set<string> = new Set(this._model.keys());
229+
let initialized = 0;
230+
231+
const changes: { pullRequestModel: PullRequestModel, status: CopilotPRStatus }[] = [];
232+
for (const folderManager of this._reposManager.folderManagers) {
233+
// It doesn't matter which repo we use since the query will specify the owner/repo.
234+
const githubRepository = folderManager.gitHubRepositories[0];
235+
if (!githubRepository) {
207236
continue;
208237
}
209-
latestEvent = CopilotPRStatus.Started;
238+
initialized++;
239+
const prs = await folderManager.getPullRequestsForCategory(githubRepository, await variableSubstitution(query, undefined, await folderManager.getPullRequestDefaults(), await this._getCurrentUser(folderManager)));
240+
for (const pr of prs?.items ?? []) {
241+
unseenKeys.delete(this._model.makeKey(pr.remote.owner, pr.remote.repositoryName, pr.number));
242+
const copilotEvents = await pr.getCopilotTimelineEvents(pr);
243+
let latestEvent = copilotEventToStatus(copilotEvents[copilotEvents.length - 1]);
244+
if (latestEvent === CopilotPRStatus.None) {
245+
if (!COPILOT_ACCOUNTS[pr.author.login]) {
246+
continue;
247+
}
248+
latestEvent = CopilotPRStatus.Started;
249+
}
250+
const lastStatus = this._model.get(pr.remote.owner, pr.remote.repositoryName, pr.number) ?? CopilotPRStatus.None;
251+
if (latestEvent !== lastStatus) {
252+
changes.push({ pullRequestModel: pr, status: latestEvent });
253+
}
254+
}
255+
256+
for (const key of unseenKeys) {
257+
this._model.deleteKey(key);
258+
}
210259
}
211-
const lastStatus = this._model.get(pr.remote.owner, pr.remote.repositoryName, pr.number) ?? CopilotPRStatus.None;
212-
if (latestEvent !== lastStatus) {
213-
stateChanges.push({
214-
owner: pr.remote.owner,
215-
repo: pr.remote.repositoryName,
216-
prNumber: pr.number,
217-
status: latestEvent
218-
});
219-
changes.push({ pullRequestModel: pr, status: latestEvent });
260+
this._model.set(changes);
261+
if (!this._model.isInitialized) {
262+
if ((initialized === this._reposManager.folderManagers.length) && (this._reposManager.folderManagers.length > 0)) {
263+
this._model.setInitialized();
264+
}
265+
return true;
266+
} else {
267+
return true;
220268
}
269+
} finally {
270+
// Ensure the stored promise is cleared so subsequent calls start a new run
271+
this._getStateChangesPromise = undefined;
221272
}
273+
})();
222274

223-
for (const key of unseenKeys) {
224-
this._model.deleteKey(key);
225-
}
226-
}
227-
this._model.set(changes);
228-
if (!this._model.isInitialized) {
229-
if ((initialized === this._reposManager.folderManagers.length) && (this._reposManager.folderManagers.length > 0)) {
230-
this._model.setInitialized();
231-
}
232-
return true;
233-
} else {
234-
return true;
235-
}
275+
return this._getStateChangesPromise;
236276
}
237277
}

src/github/copilotRemoteAgent.ts

Lines changed: 14 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ import { CODING_AGENT, CODING_AGENT_AUTO_COMMIT_AND_PUSH } from '../common/setti
1818
import { ITelemetry } from '../common/telemetry';
1919
import { toOpenPullRequestWebviewUri } from '../common/uri';
2020
import { dateFromNow } from '../common/utils';
21-
import { copilotEventToSessionStatus, IAPISessionLogs, ICopilotRemoteAgentCommandArgs, ICopilotRemoteAgentCommandResponse, OctokitCommon, RemoteAgentResult, RepoInfo } from './common';
21+
import { copilotEventToSessionStatus, copilotPRStatusToSessionStatus, IAPISessionLogs, ICopilotRemoteAgentCommandArgs, ICopilotRemoteAgentCommandResponse, OctokitCommon, RemoteAgentResult, RepoInfo } from './common';
2222
import { ChatSessionWithPR, CopilotApi, getCopilotApi, RemoteAgentJobPayload, SessionInfo, SessionSetupStep } from './copilotApi';
2323
import { CopilotPRWatcher, CopilotStateModel } from './copilotPrWatcher';
2424
import { ChatSessionContentBuilder } from './copilotRemoteAgent/chatSessionContentBuilder';
@@ -72,7 +72,10 @@ export class CopilotRemoteAgentManager extends Disposable {
7272

7373
this._stateModel = new CopilotStateModel();
7474
this._register(new CopilotPRWatcher(this.repositoriesManager, this._stateModel));
75-
this._register(this._stateModel.onDidChangeStates(() => this._onDidChangeStates.fire()));
75+
this._register(this._stateModel.onDidChangeStates(() => {
76+
this._onDidChangeStates.fire();
77+
this._onDidChangeChatSessions.fire();
78+
}));
7679
this._register(this._stateModel.onDidChangeNotifications(items => this._onDidChangeNotifications.fire(items)));
7780

7881
this._register(this.repositoriesManager.onDidChangeFolderRepositories((event) => {
@@ -774,17 +777,17 @@ export class CopilotRemoteAgentManager extends Disposable {
774777

775778
await this.waitRepoManagerInitialization();
776779

777-
const codingAgentPRs = await capi.getAllCodingAgentPRs(this.repositoriesManager);
778-
return await Promise.all(codingAgentPRs.map(async session => {
779-
const timeline = await session.getCopilotTimelineEvents(session);
780-
const status = copilotEventToSessionStatus(mostRecentCopilotEvent(timeline));
781-
const tooltip = await issueMarkdown(session, this.context, this.repositoriesManager);
780+
const codingAgentPRs = this._stateModel.all;
781+
return await Promise.all(codingAgentPRs.map(async prAndStatus => {
782+
const status = copilotPRStatusToSessionStatus(prAndStatus.status);
783+
const pullRequest = prAndStatus.item;
784+
const tooltip = await issueMarkdown(pullRequest, this.context, this.repositoriesManager);
782785
return {
783-
id: `${session.number}`,
784-
label: session.title || `Session ${session.number}`,
786+
id: `${pullRequest.number}`,
787+
label: pullRequest.title || `Session ${pullRequest.number}`,
785788
iconPath: this.getIconForSession(status),
786-
description: `${dateFromNow(session.createdAt)}`,
787-
pullRequest: session,
789+
description: `${dateFromNow(pullRequest.createdAt)}`,
790+
pullRequest: pullRequest,
788791
tooltip,
789792
status,
790793
};

0 commit comments

Comments
 (0)