Skip to content
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,7 @@ export function buildSessionPullRequestSections(pullRequests: readonly IResolved
})],
...getChatPillResourceLocation(ref.uri, label),
ariaDescription: localize('sessionChatPills.pullRequestDescription', "{0}. {1}", stateDescription, ref.uri.toString(true)),
...(!pullRequest && ref.title ? { tooltip: `${label}\n${ref.uri.toString(true)}` } : {}),
...(pullRequest ? {
pillHover: {
element: () => createPullRequestHoverElement({
Expand Down Expand Up @@ -142,8 +143,9 @@ interface IResolvedSessionIssue {
/** Builds Agents Window issue pill entries, enriching them when live details are available. */
export function buildSessionIssueSections(issues: readonly IResolvedSessionIssue[], session: IActiveSession | undefined, commandService: ICommandService, clipboardService: IClipboardService, openerService: IOpenerService, sessionsService: ISessionsService): readonly IChatPillSection[] {
const entries = issues.map(({ ref, issue }) => {
const label = issue?.title
? localize('sessionChatPills.issueWithTitle', "Issue #{0}: {1}", ref.number, issue.title)
const title = issue?.title ?? ref.title;
const label = title
? localize('sessionChatPills.issueWithTitle', "Issue #{0}: {1}", ref.number, title)
: localize('sessionChatPills.issue', "Issue #{0}", ref.number);
return {
id: ref.uri.toString(),
Expand All @@ -157,6 +159,7 @@ export function buildSessionIssueSections(issues: readonly IResolvedSessionIssue
run: () => clipboardService.writeText(ref.uri.toString(true)),
})],
...getChatPillResourceLocation(ref.uri, label),
...(!issue && ref.title ? { tooltip: `${label}\n${ref.uri.toString(true)}` } : {}),
...(issue ? {
pillHover: {
element: () => createIssueHoverElement({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,13 @@ suite('SessionChatInputToolbar', () => {
}

test('adds rich GitHub hovers only when live details are available', async () => {
const commandService = upcastPartial<ICommandService>({ executeCommand: async () => undefined });
const commands: { readonly id: string; readonly args: readonly unknown[] }[] = [];
const commandService = upcastPartial<ICommandService>({
executeCommand: async (id, ...args) => {
commands.push({ id, args });
return undefined;
},
});
const clipboardService = upcastPartial<IClipboardService>({ writeText: async () => { } });
const openerService = upcastPartial<IOpenerService>({ open: async () => true });
const sessionsService = upcastPartial<ISessionsService>({ setActive: () => { } });
Expand All @@ -149,6 +155,7 @@ suite('SessionChatInputToolbar', () => {
repo: 'vscode',
number: 332982,
uri: URI.parse('https://github.com/microsoft/vscode/pull/332982'),
title: 'Recorded pull request title',
};
const pullRequest: IGitHubPullRequest = {
number: pullRequestRef.number,
Expand All @@ -171,6 +178,7 @@ suite('SessionChatInputToolbar', () => {
repo: 'vscode',
number: 42,
uri: URI.parse('https://github.com/microsoft/vscode/issues/42'),
title: 'Recorded issue title',
};
const issue: IGitHubIssue = {
number: issueRef.number,
Expand Down Expand Up @@ -224,38 +232,67 @@ suite('SessionChatInputToolbar', () => {
};
const pullRequestHover = await renderHover(pullRequestEntry);
const issueHover = await renderHover(issueEntry);
pullRequestEntry?.open();
unresolvedIssueEntry?.open();

assert.deepStrictEqual({
pullRequest: {
label: pullRequestEntry?.label,
className: pullRequestHover?.className,
repository: pullRequestHover?.querySelector('.sessions-pr-hover-repository')?.textContent,
title: pullRequestHover?.querySelector('.sessions-pr-hover-title')?.textContent,
description: pullRequestHover?.querySelector('.sessions-pr-hover-description-content')?.textContent,
branches: [...pullRequestHover?.querySelectorAll('.sessions-pr-hover-branch') ?? []].map(element => element.textContent),
unresolvedLabel: unresolvedPullRequestEntry?.label,
unresolvedAriaLabel: unresolvedPullRequestEntry?.ariaLabel,
unresolvedTooltip: unresolvedPullRequestEntry?.tooltip,
unresolvedHover: unresolvedPullRequestEntry?.pillHover,
},
issue: {
label: issueEntry?.label,
className: issueHover?.className,
repository: issueHover?.querySelector('.sessions-issue-hover-repository')?.textContent,
title: issueHover?.querySelector('.sessions-issue-hover-title')?.textContent,
description: issueHover?.querySelector('.sessions-issue-hover-description-content')?.textContent,
unresolvedLabel: unresolvedIssueEntry?.label,
unresolvedAriaLabel: unresolvedIssueEntry?.ariaLabel,
unresolvedTooltip: unresolvedIssueEntry?.tooltip,
unresolvedHover: unresolvedIssueEntry?.pillHover,
openCommands: commands,
},
}, {
pullRequest: {
label: 'Pull Request #332982: Restore rich pill hovers',
className: 'sessions-pr-hover',
repository: 'microsoft/vscode',
title: 'Restore rich pill hovers',
description: 'Provides detailed pull request context.',
branches: ['main', 'feature/rich-hover'],
unresolvedLabel: 'Pull Request #332982: Recorded pull request title',
unresolvedAriaLabel: 'Open Pull Request #332982: Recorded pull request title',
unresolvedTooltip: 'Pull Request #332982: Recorded pull request title\nhttps://github.com/microsoft/vscode/pull/332982',
unresolvedHover: undefined,
},
issue: {
label: 'Issue #42: Rich issue hover',
className: 'sessions-issue-hover',
repository: 'microsoft/vscode#42',
title: 'Rich issue hover',
description: 'Provides detailed issue context.',
unresolvedLabel: 'Issue #42: Recorded issue title',
unresolvedAriaLabel: 'Open Issue #42: Recorded issue title',
unresolvedTooltip: 'Issue #42: Recorded issue title\nhttps://github.com/microsoft/vscode/issues/42',
unresolvedHover: undefined,
openCommands: [
{
id: 'workbench.agentSessions.action.openPullRequest',
args: [{ pullRequest: pullRequestRef }],
},
{
id: 'workbench.agentSessions.action.openIssue',
args: [{ issue: issueRef }],
},
],
},
});
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,8 @@ export interface ISessionArtifactPartition {
readonly pullRequestTitles: ReadonlyMap<string, string>;
/** Issues this session produced, most recent first. */
readonly issueUrls: readonly string[];
/** Titles the agent recorded for its issue artifacts, keyed by {@link linkKey}. */
readonly issueTitles: ReadonlyMap<string, string>;
}

interface ISessionArtifactEntry {
Expand Down Expand Up @@ -98,6 +100,7 @@ export function partitionSessionArtifacts(meta: SessionMeta | undefined): ISessi
const pullRequestUrls: string[] = [];
const pullRequestTitles = new Map<string, string>();
const issueUrls: string[] = [];
const issueTitles = new Map<string, string>();

for (const artifact of readSessionArtifacts(meta)) {
const mapped = toSessionArtifact(artifact);
Expand All @@ -110,17 +113,17 @@ export function partitionSessionArtifacts(meta: SessionMeta | undefined): ISessi
continue;
}

const titles = artifact.type === SessionArtifactType.Issue ? issueTitles : pullRequestTitles;
const key = linkKey(link);
if (mapped.label && !titles.has(key)) {
titles.set(key, mapped.label);
}

if (artifact.type === SessionArtifactType.Issue) {
issueUrls.push(link);
continue;
}

// The label an agent records for a pull request is its title; keep the
// first one so a later duplicate cannot rewrite it.
const key = linkKey(link);
if (mapped.label && !pullRequestTitles.has(key)) {
pullRequestTitles.set(key, mapped.label);
}
pullRequestUrls.push(link);
}

Expand All @@ -129,7 +132,7 @@ export function partitionSessionArtifacts(meta: SessionMeta | undefined): ISessi
pullRequestUrls.reverse();
issueUrls.reverse();

return { entries, pullRequestUrls, pullRequestTitles, issueUrls };
return { entries, pullRequestUrls, pullRequestTitles, issueUrls, issueTitles };
}

/** Case-insensitive de-duplication that keeps the first occurrence's casing. */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -336,7 +336,12 @@ function isGitHubInfoEqual(a: IGitHubInfo | undefined, b: IGitHubInfo | undefine
a.pullRequest?.title === b.pullRequest?.title &&
a.pullRequest?.baseRefOid === b.pullRequest?.baseRefOid &&
a.pullRequest?.headRefOid === b.pullRequest?.headRefOid &&
arrayEquals(a.issues ?? [], b.issues ?? [], (x, y) => x.owner === y.owner && x.repo === y.repo && x.number === y.number);
arrayEquals(a.issues ?? [], b.issues ?? [], (x, y) =>
x.owner === y.owner &&
x.repo === y.repo &&
x.number === y.number &&
isEqual(x.uri, y.uri) &&
x.title === y.title);
}

function dateEquals(a: Date | undefined, b: Date | undefined): boolean {
Expand All @@ -348,12 +353,17 @@ function markdownStringEquals(a: IMarkdownString | undefined, b: IMarkdownString
}

/** Maps the GitHub issue URLs recorded on the session's metadata to issue references. */
function toGitHubIssueRefs(issueUrls: readonly string[] | undefined): readonly IGitHubIssueRef[] | undefined {
function toGitHubIssueRefs(issueUrls: readonly string[] | undefined, titles: ReadonlyMap<string, string>): readonly IGitHubIssueRef[] | undefined {
const refs: IGitHubIssueRef[] = [];
for (const url of issueUrls ?? []) {
const reference = parseGitHubIssueUrl(url);
if (reference) {
refs.push({ ...reference, uri: URI.parse(url) });
const title = titles.get(linkKey(url));
refs.push({
...reference,
uri: URI.parse(url),
...(title ? { title } : {}),
});
}
}
return refs.length > 0 ? refs : undefined;
Expand Down Expand Up @@ -387,7 +397,7 @@ function toGitHubPullRequestRefs(state: ISessionGitHubState | undefined, pullReq
function toGitHubInfo(meta: SessionMeta | undefined): IGitHubInfo | undefined {
const state = readSessionGitHubState(meta);
const gitState = readSessionGitState(meta);
const { pullRequestUrls, pullRequestTitles, issueUrls } = partitionSessionArtifacts(meta);
const { pullRequestUrls, pullRequestTitles, issueUrls, issueTitles } = partitionSessionArtifacts(meta);

// Recorded pull requests lead discovered ones, so the first is the newest.
const allPullRequests = toGitHubPullRequestRefs(state, dedupeLinks(pullRequestUrls, getSessionRelatedPullRequestUrls(state)), pullRequestTitles);
Expand All @@ -408,7 +418,7 @@ function toGitHubInfo(meta: SessionMeta | undefined): IGitHubInfo | undefined {

const pullRequests = allPullRequests?.filter(belongsToRepository);
const pullRequest = pullRequests?.at(0);
const issues = toGitHubIssueRefs(dedupeLinks(issueUrls))?.filter(belongsToRepository);
const issues = toGitHubIssueRefs(dedupeLinks(issueUrls), issueTitles)?.filter(belongsToRepository);

return {
owner: repository.owner,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7824,7 +7824,7 @@ suite('LocalAgentHostSessionsProvider', () => {
{ id: 'a1', type: SessionArtifactType.PullRequest, label: 'Created', isArtifact: true, link: 'https://github.com/owner/repo/pull/50', isGitHub: true },
{ id: 'a2', type: SessionArtifactType.PullRequest, label: 'Referenced', isArtifact: false, link: 'https://github.com/owner/repo/pull/60', isGitHub: true },
{ id: 'a3', type: SessionArtifactType.PullRequest, label: 'Duplicate', isArtifact: true, link: 'https://github.com/OWNER/REPO/pull/41/', isGitHub: true },
{ id: 'a4', type: SessionArtifactType.Issue, label: 'Issue', isArtifact: true, link: 'https://github.com/owner/repo/issues/7', isGitHub: true },
{ id: 'a4', type: SessionArtifactType.Issue, label: 'Preserve promoted issue titles', isArtifact: true, link: 'https://github.com/owner/repo/issues/7', isGitHub: true },
{ id: 'a5', type: SessionArtifactType.PullRequest, label: 'Elsewhere', isArtifact: true, link: 'https://gitlab.com/owner/repo/-/merge_requests/3', isGitHub: false },
{ id: 'a6', type: SessionArtifactType.File, label: 'Plan', isArtifact: true, uri: 'file:///repo/plan.md' },
{ id: 'a7', type: SessionArtifactType.Issue, label: 'Referenced issue', isArtifact: false, link: 'https://github.com/owner/repo/issues/8', isGitHub: true },
Expand All @@ -7842,13 +7842,13 @@ suite('LocalAgentHostSessionsProvider', () => {
assert.deepStrictEqual({
activePullRequest: gitHubInfo?.pullRequest?.number,
pullRequests: gitHubInfo?.pullRequests?.map(pullRequest => pullRequest.number),
issues: gitHubInfo?.issues?.map(issue => issue.number),
issues: gitHubInfo?.issues?.map(issue => [issue.number, issue.title]),
artifacts: session.artifacts?.get().map(artifact => [artifact.id, artifact.isArtifact]),
}, {
activePullRequest: 41,
pullRequests: [41, 50, 42],
// Only issues the session produced are polled; a referenced one stays a reference.
issues: [7],
issues: [[7, 'Preserve promoted issue titles']],
artifacts: [
['a8', false],
['a7', false],
Expand Down
10 changes: 9 additions & 1 deletion src/vs/sessions/services/sessions/common/session.ts
Original file line number Diff line number Diff line change
Expand Up @@ -404,6 +404,8 @@ export interface IGitHubIssueRef {
readonly number: number;
/** URI of the issue. */
readonly uri: URI;
/** Issue title recorded by the session, when known. */
readonly title?: string;
}

export interface ISessionChangesSummary {
Expand Down Expand Up @@ -1050,7 +1052,13 @@ export function gitHubInfoEqual(a: IGitHubInfo | undefined, b: IGitHubInfo | und
(aIcon === bIcon || (!!aIcon && !!bIcon && ThemeIcon.isEqual(aIcon, bIcon))) &&
a.pullRequest?.title === b.pullRequest?.title &&
a.pullRequest?.baseRefOid === b.pullRequest?.baseRefOid &&
a.pullRequest?.headRefOid === b.pullRequest?.headRefOid;
a.pullRequest?.headRefOid === b.pullRequest?.headRefOid &&
arrayEquals(a.issues ?? [], b.issues ?? [], (x, y) =>
x.owner === y.owner &&
x.repo === y.repo &&
x.number === y.number &&
isEqual(x.uri, y.uri) &&
x.title === y.title);
}

/**
Expand Down
17 changes: 17 additions & 0 deletions src/vs/sessions/services/sessions/test/common/session.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,23 @@ suite('sessionWorkspaceEqual', () => {
assert.strictEqual(sessionWorkspaceEqual(workspace('main', constObservable(gitHubInfoA)), workspace('main', constObservable(gitHubInfoB))), true);
});

test('compares recorded issue titles in GitHub info', () => {
const uri = URI.parse('https://github.com/owner/repo/issues/42');
const base: IGitHubInfo = {
owner: 'owner',
repo: 'repo',
issues: [{ owner: 'owner', repo: 'repo', number: 42, uri, title: 'Recorded title' }],
};

assert.deepStrictEqual({
equivalent: sessionWorkspaceEqual(workspace('main', constObservable(base)), workspace('main', constObservable({ ...base, issues: [{ ...base.issues![0] }] }))),
changedTitle: sessionWorkspaceEqual(workspace('main', constObservable(base)), workspace('main', constObservable({ ...base, issues: [{ ...base.issues![0], title: 'Updated title' }] }))),
}, {
equivalent: true,
changedTitle: false,
});
});

test('returns false when folder repository metadata changes', () => {
assert.strictEqual(sessionWorkspaceEqual(workspace('main'), workspace('feature')), false);
});
Expand Down
2 changes: 1 addition & 1 deletion src/vs/workbench/browser/chatDropdownPill.ts
Original file line number Diff line number Diff line change
Expand Up @@ -298,7 +298,7 @@ export class ChatDropdownPillActionViewItem extends ChatPillActionViewItem {
getAriaLabel: item => item.label ?? '',
getWidgetAriaLabel: () => this._pillOptions.title,
},
{ minWidth: 240, maxWidth: 460, widgetClassName: 'show-file-icons' },
{ minWidth: 240, maxWidth: 460, widgetClassName: 'show-file-icons chat-pill-dropdown' },
);
}

Expand Down
8 changes: 8 additions & 0 deletions src/vs/workbench/browser/media/chatPills.css
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,14 @@
background-position: center center;
}

.action-widget.chat-pill-dropdown .monaco-list .monaco-list-row.has-toolbar:not(.has-standalone-toggle):not(.has-inline-toggle):not(.has-detail) {
padding-right: 0;
}

.action-widget.chat-pill-dropdown .monaco-list .monaco-list-row.has-toolbar:not(.has-standalone-toggle):not(.has-inline-toggle):not(.has-detail) .action-list-item-toolbar {
margin-right: 0;
}

/* Horizontally scrollable status pills above a chat input. */
.chat-pills-row {
width: 100%;
Expand Down
Loading
Loading