Skip to content

Commit 5618f96

Browse files
committed
Move details to hover (#315215)
1 parent 873852c commit 5618f96

5 files changed

Lines changed: 303 additions & 108 deletions

File tree

‎src/vs/workbench/contrib/chat/browser/chat.contribution.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1023,7 +1023,7 @@ configurationRegistry.registerConfiguration({
10231023
[ChatConfiguration.ToolRiskAssessmentEnabled]: {
10241024
type: 'boolean',
10251025
description: nls.localize('chat.tools.riskAssessment.enabled', "When enabled, terminal tool confirmations show an LLM-generated risk level (Safe / Caution / Review carefully) and a short explanation."),
1026-
default: false,
1026+
default: true,
10271027
tags: ['experimental'],
10281028
experiment: {
10291029
mode: 'auto'

‎src/vs/workbench/contrib/chat/browser/widget/chatContentParts/toolInvocationParts/chatTerminalToolConfirmationSubPart.ts‎

Lines changed: 73 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ import { asArray } from '../../../../../../../base/common/arrays.js';
1111
import { CancellationTokenSource } from '../../../../../../../base/common/cancellation.js';
1212
import { Codicon } from '../../../../../../../base/common/codicons.js';
1313
import { ErrorNoTelemetry } from '../../../../../../../base/common/errors.js';
14-
import { createCommandUri, MarkdownString, type IMarkdownString } from '../../../../../../../base/common/htmlContent.js';
14+
import { createCommandUri, escapeMarkdownSyntaxTokens, MarkdownString, type IMarkdownString } from '../../../../../../../base/common/htmlContent.js';
1515
import { toDisposable } from '../../../../../../../base/common/lifecycle.js';
1616
import Severity from '../../../../../../../base/common/severity.js';
1717
import { isObject } from '../../../../../../../base/common/types.js';
@@ -191,29 +191,92 @@ export class ChatTerminalToolConfirmationSubPart extends BaseChatToolInvocationS
191191
position: { hoverPosition: HoverPosition.LEFT },
192192
}));
193193

194+
const riskBadge = this._createRiskBadge(state.parameters);
195+
194196
const confirmWidget = this._register(this.instantiationService.createInstance(
195197
ChatCustomConfirmationWidget<TerminalNewAutoApproveButtonData | boolean>,
196198
this.context,
197199
{
198200
title,
199201
icon: Codicon.terminal,
200202
message: elements.root,
201-
footerBanner: this._createRiskBadge(state.parameters),
203+
footerBanner: riskBadge?.domNode,
202204
buttons: this._createButtons(moreActions)
203205
},
204206
));
205207

208+
// Build the unsandboxed-execution reason and disclaimer markdown. When
209+
// the risk badge is shown, surface them via its details hover (with
210+
// labelled prefixes) instead of the dedicated disclaimer row to keep
211+
// the confirmation compact.
212+
interface IDetailPart {
213+
readonly inline: IMarkdownString;
214+
readonly hoverLabel: string;
215+
readonly hoverBody: string;
216+
readonly isTrusted: IMarkdownString['isTrusted'];
217+
}
218+
const detailParts: IDetailPart[] = [];
206219
if (terminalData.requestUnsandboxedExecution) {
207220
const reasonText = (terminalData.requestUnsandboxedExecutionReason && terminalData.requestUnsandboxedExecutionReason.trim())
208221
|| localize('chat.terminal.unsandboxedExecution.defaultReason', "The model did not provide a reason for requesting unsandboxed execution.");
209-
const unsandboxedReasonMarkdown = new MarkdownString(undefined, { supportThemeIcons: true });
210-
unsandboxedReasonMarkdown.appendMarkdown(`$(${Codicon.info.id}) `);
211-
unsandboxedReasonMarkdown.appendText(reasonText);
212-
this._appendMarkdownPart(elements.disclaimer, unsandboxedReasonMarkdown, codeBlockRenderOptions);
222+
const inline = new MarkdownString(undefined, { supportThemeIcons: true });
223+
inline.appendMarkdown(`$(${Codicon.info.id}) `);
224+
inline.appendText(reasonText);
225+
detailParts.push({
226+
inline,
227+
hoverLabel: localize('chat.terminal.detail.sandboxInsufficient', "Sandbox insufficient:"),
228+
hoverBody: escapeMarkdownSyntaxTokens(reasonText),
229+
isTrusted: undefined,
230+
});
213231
}
214-
215232
if (disclaimer) {
216-
this._appendMarkdownPart(elements.disclaimer, disclaimer, codeBlockRenderOptions);
233+
const inline = typeof disclaimer === 'string' ? new MarkdownString(disclaimer) : disclaimer;
234+
// For the hover, drop the leading `$(info) ` icon prefix that the
235+
// disclaimer carries for inline rendering — the labelled prefix
236+
// already conveys the same role.
237+
const hoverBody = inline.value.replace(/^\s*\$\([^)]+\)\s*/, '');
238+
detailParts.push({
239+
inline,
240+
hoverLabel: localize('chat.terminal.detail.approvalNeeded', "Approval needed:"),
241+
hoverBody,
242+
isTrusted: inline.isTrusted,
243+
});
244+
}
245+
246+
const renderInlineDisclaimers = () => {
247+
elements.disclaimer.replaceChildren();
248+
for (const part of detailParts) {
249+
this._appendMarkdownPart(elements.disclaimer, part.inline, codeBlockRenderOptions);
250+
}
251+
};
252+
253+
if (riskBadge && detailParts.length) {
254+
const combined = new MarkdownString(undefined, {
255+
supportThemeIcons: true,
256+
isTrusted: detailParts.reduce<MarkdownString['isTrusted']>((acc, part) => {
257+
if (part.isTrusted === true || acc === true) {
258+
return true;
259+
}
260+
if (typeof part.isTrusted === 'object' && part.isTrusted) {
261+
const enabled = new Set([
262+
...(typeof acc === 'object' && acc?.enabledCommands ? acc.enabledCommands : []),
263+
...part.isTrusted.enabledCommands,
264+
]);
265+
return { enabledCommands: [...enabled] };
266+
}
267+
return acc;
268+
}, undefined),
269+
});
270+
detailParts.forEach((part, i) => {
271+
if (i > 0) {
272+
combined.appendMarkdown('\n\n');
273+
}
274+
combined.appendMarkdown(`**${escapeMarkdownSyntaxTokens(part.hoverLabel)}** ${part.hoverBody}`);
275+
});
276+
riskBadge.setDetails(combined);
277+
this._register(riskBadge.onDidHide(() => renderInlineDisclaimers()));
278+
} else {
279+
renderInlineDisclaimers();
217280
}
218281

219282
const hasToolConfirmationKey = ChatContextKeys.Editing.hasToolConfirmation.bindTo(this.contextKeyService);
@@ -431,7 +494,7 @@ export class ChatTerminalToolConfirmationSubPart extends BaseChatToolInvocationS
431494
return promptResult.result === true;
432495
}
433496

434-
private _createRiskBadge(parameters: unknown): HTMLElement | undefined {
497+
private _createRiskBadge(parameters: unknown): ToolRiskBadgeWidget | undefined {
435498
if (!this.riskAssessmentService.isEnabled()) {
436499
return undefined;
437500
}
@@ -462,7 +525,7 @@ export class ChatTerminalToolConfirmationSubPart extends BaseChatToolInvocationS
462525
}
463526
})();
464527
}
465-
return widget.domNode;
528+
return widget;
466529
}
467530

468531
private _appendMarkdownPart(container: HTMLElement, message: string | IMarkdownString, codeBlockRenderOptions: ICodeBlockRenderOptions) {

‎src/vs/workbench/contrib/chat/browser/widget/chatContentParts/toolInvocationParts/media/toolRiskBadge.css‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,23 @@
4343
white-space: nowrap;
4444
}
4545

46+
.chat-confirmation-widget2 > .tool-risk-badge .tool-risk-details-icon {
47+
flex: 0 0 auto;
48+
display: inline-flex;
49+
align-items: center;
50+
justify-content: center;
51+
font-size: 13px;
52+
line-height: 1;
53+
color: var(--vscode-descriptionForeground);
54+
cursor: pointer;
55+
border-radius: 3px;
56+
}
57+
58+
.chat-confirmation-widget2 > .tool-risk-badge .tool-risk-details-icon:focus {
59+
outline: 1px solid var(--vscode-focusBorder);
60+
outline-offset: 1px;
61+
}
62+
4663
.chat-confirmation-widget2 > .tool-risk-badge.loading {
4764
opacity: 0.7;
4865
}

‎src/vs/workbench/contrib/chat/browser/widget/chatContentParts/toolInvocationParts/toolRiskBadgeWidget.ts‎

Lines changed: 64 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@
55

66
import * as dom from '../../../../../../../base/browser/dom.js';
77
import { Codicon } from '../../../../../../../base/common/codicons.js';
8+
import { Emitter, Event } from '../../../../../../../base/common/event.js';
9+
import { IMarkdownString, MarkdownString } from '../../../../../../../base/common/htmlContent.js';
810
import { Disposable, DisposableStore } from '../../../../../../../base/common/lifecycle.js';
911
import { ThemeIcon } from '../../../../../../../base/common/themables.js';
1012
import { localize } from '../../../../../../../nls.js';
@@ -22,7 +24,13 @@ export class ToolRiskBadgeWidget extends Disposable {
2224

2325
private readonly _iconEl: HTMLElement;
2426
private readonly _textEl: HTMLElement;
27+
private readonly _detailsIconEl: HTMLElement;
2528
private readonly _hoverStore = this._register(new DisposableStore());
29+
private readonly _detailsHoverStore = this._register(new DisposableStore());
30+
private _details: IMarkdownString | string | undefined;
31+
32+
private readonly _onDidHide = this._register(new Emitter<void>());
33+
public readonly onDidHide: Event<void> = this._onDidHide.event;
2634

2735
constructor(
2836
@IHoverService private readonly _hoverService: IHoverService,
@@ -32,7 +40,13 @@ export class ToolRiskBadgeWidget extends Disposable {
3240
this.domNode = dom.$(`span.${RISK_BADGE_CLASS}`);
3341
this._iconEl = dom.$('span.tool-risk-icon');
3442
this._textEl = dom.$('span.tool-risk-text');
35-
this.domNode.append(this._iconEl, this._textEl);
43+
this._detailsIconEl = dom.$('span.tool-risk-details-icon');
44+
this._detailsIconEl.classList.add(...ThemeIcon.asClassNameArray(Codicon.info));
45+
this._detailsIconEl.tabIndex = 0;
46+
this._detailsIconEl.setAttribute('role', 'button');
47+
this._detailsIconEl.setAttribute('aria-label', localize('toolRisk.detailsIconLabel', "Risk assessment details"));
48+
this.domNode.append(this._iconEl, this._textEl, this._detailsIconEl);
49+
this._refreshDetailsHover();
3650
this.setLoading();
3751
}
3852

@@ -46,6 +60,7 @@ export class ToolRiskBadgeWidget extends Disposable {
4660

4761
setHidden(): void {
4862
this.domNode.style.display = 'none';
63+
this._onDidHide.fire();
4964
}
5065

5166
setAssessment(assessment: IToolRiskAssessment): void {
@@ -68,6 +83,24 @@ export class ToolRiskBadgeWidget extends Disposable {
6883
this._setHover(assessment.explanation);
6984
}
7085

86+
/**
87+
* Provide additional context to surface in the trailing info icon's hover.
88+
* The hover always notes that the assessment is AI-generated; any details
89+
* passed here are appended below that note.
90+
*/
91+
setDetails(details: IMarkdownString | string | undefined): void {
92+
this._details = details;
93+
this._refreshDetailsHover();
94+
}
95+
96+
/**
97+
* The markdown content currently shown in the trailing info icon's hover.
98+
* Exposed so component fixtures can render a preview of the hover content.
99+
*/
100+
getDetailsMarkdown(): IMarkdownString {
101+
return this._buildDetailsMarkdown();
102+
}
103+
71104
private _setVariant(variant: 'loading' | 'green' | 'orange' | 'red'): void {
72105
this.domNode.classList.remove('green', 'orange', 'red', 'loading');
73106
this.domNode.classList.add(variant);
@@ -87,4 +120,34 @@ export class ToolRiskBadgeWidget extends Disposable {
87120
this._hoverStore.clear();
88121
this._hoverStore.add(this._hoverService.setupManagedHover(getDefaultHoverDelegate('mouse'), this.domNode, content));
89122
}
123+
124+
private _refreshDetailsHover(): void {
125+
this._detailsHoverStore.clear();
126+
const md = this._buildDetailsMarkdown();
127+
const fallback = md.value.replace(/\$\([^)]+\)\s?/g, '');
128+
this._detailsHoverStore.add(this._hoverService.setupManagedHover(
129+
getDefaultHoverDelegate('element'),
130+
this._detailsIconEl,
131+
{ markdown: md, markdownNotSupportedFallback: fallback },
132+
));
133+
}
134+
135+
private _buildDetailsMarkdown(): IMarkdownString {
136+
const aiNote = localize('toolRisk.aiGenerated', "Risk assessments are AI-generated and may be inaccurate.");
137+
const details = this._details;
138+
const md = new MarkdownString(undefined, {
139+
supportThemeIcons: true,
140+
isTrusted: typeof details === 'object' && details ? details.isTrusted : undefined,
141+
});
142+
md.appendText(aiNote);
143+
if (details) {
144+
md.appendMarkdown('\n\n');
145+
if (typeof details === 'string') {
146+
md.appendText(details);
147+
} else {
148+
md.appendMarkdown(details.value);
149+
}
150+
}
151+
return md;
152+
}
90153
}

0 commit comments

Comments
 (0)