-
Notifications
You must be signed in to change notification settings - Fork 37.7k
feat: add vision policy handling for image attachments #292586
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Adds organization policy awareness to chat image attachment rendering so users get a warning (and no preview) when preview features are disabled by policy.
Changes:
- Inject
IDefaultAccountServiceinto image-related attachment widgets to read account policy data. - Add a policy check (
chat_preview_features_enabled === false) to drive warning state for image attachments. - Update image attachment pill/hover behavior to show a policy-based warning.
Comments suppressed due to low confidence (2)
src/vs/workbench/contrib/chat/browser/attachments/chatAttachmentWidgets.ts:471
- The hover text shown when the policy disables previews says "Vision is disabled by your organization", but the underlying policy flag is
chat_preview_features_enabled(editor preview features). Consider rewording this message to match what is actually blocked here (image previews in chat) and/or that it’s a policy-controlled restriction, to avoid confusing users.
if (visionDisabledByPolicy) {
element.classList.add('warning');
hoverElement.textContent = localize('chat.visionDisabledByOrganization', "Vision is disabled by your organization");
disposable.add(hoverService.setupDelayedHover(element, {
content: hoverElement,
style: HoverStyle.Pointer,
}));
src/vs/workbench/contrib/chat/browser/attachments/chatAttachmentWidgets.ts:834
NotebookCellOutputChatAttachmentWidgetnow requiresIDefaultAccountServicetoo, which has the same impact on test and standalone instantiation asImageAttachmentWidget(the standardworkbenchInstantiationServicedoesn’t provide this service today). Please ensure a stub is registered in test service collections (or make the dependency optional with a safe default) so widget creation can’t throw due to a missing service.
@INotebookService private readonly notebookService: INotebookService,
@IInstantiationService private readonly instantiationService: IInstantiationService,
@IDefaultAccountService private readonly defaultAccountService: IDefaultAccountService,
) {
src/vs/workbench/contrib/chat/browser/attachments/chatAttachmentWidgets.ts
Show resolved
Hide resolved
src/vs/workbench/contrib/chat/browser/attachments/chatAttachmentWidgets.ts
Show resolved
Hide resolved
src/vs/workbench/contrib/chat/browser/attachments/chatAttachmentWidgets.ts
Show resolved
Hide resolved
|
@cwebster-99 I've opened a new pull request, #292594, to work on those changes. Once the pull request is ready, I'll request review from you. |
Made IDefaultAccountService optional in ImageAttachmentWidget and NotebookCellOutputChatAttachmentWidget to prevent test failures when the service is not stubbed. This follows the existing pattern with ITerminalService and treats missing policy data as "enabled" (vision allowed). Co-authored-by: cwebster-99 <60238438+cwebster-99@users.noreply.github.com>
Make IDefaultAccountService optional in image attachment widgets
Adds warning on image attachments when editor preview features is disabled
