Skip to content

Commit 8dab8ae

Browse files
travkin79travkin79
committed
feat: add preference for a chat's custom instructions loading
1 parent b303976 commit 8dab8ae

14 files changed

Lines changed: 475 additions & 10 deletions

File tree

‎com.microsoft.copilot.eclipse.core.test/src/com/microsoft/copilot/eclipse/core/CustomInstructionsChatLoadScopeTest.java‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
// Copyright (c) Microsoft Corporation.
2+
// Licensed under the MIT license.
3+
4+
package com.microsoft.copilot.eclipse.core;
5+
6+
import static org.junit.jupiter.api.Assertions.assertEquals;
7+
import static org.junit.jupiter.api.Assertions.assertThrows;
8+
9+
import org.junit.jupiter.params.ParameterizedTest;
10+
import org.junit.jupiter.params.provider.EnumSource;
11+
import org.junit.jupiter.params.provider.NullSource;
12+
import org.junit.jupiter.params.provider.ValueSource;
13+
14+
import com.microsoft.copilot.eclipse.core.chat.CustomInstructionsChatLoadScope;
15+
16+
class CustomInstructionsChatLoadScopeTest {
17+
18+
@ParameterizedTest
19+
@EnumSource(CustomInstructionsChatLoadScope.class)
20+
void testStringToEnumEntryConversion(CustomInstructionsChatLoadScope enumEntry) {
21+
String inputValue = enumEntry.getValue();
22+
23+
CustomInstructionsChatLoadScope actualResult = CustomInstructionsChatLoadScope.fromValue(inputValue);
24+
25+
assertEquals(enumEntry, actualResult);
26+
}
27+
28+
@ParameterizedTest
29+
@ValueSource(strings = { "wrongValue" })
30+
@NullSource
31+
void testStringToEnumEntryConversionThrowsExceptionForWrongValues(String value) {
32+
assertThrows(IllegalArgumentException.class, () -> CustomInstructionsChatLoadScope.fromValue(value));
33+
}
34+
35+
}

‎com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/Constants.java‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,9 @@ private Constants() {
3939
public static final String CUSTOM_INSTRUCTIONS_WORKSPACE = "customInstructionsWorkspace";
4040
public static final String CUSTOM_INSTRUCTIONS_WORKSPACE_ENABLED = "customInstructionsWorkspaceEnabled";
4141
public static final String CUSTOM_INSTRUCTIONS_GIT_COMMIT = "customInstructionsGitCommit";
42+
public static final String CUSTOM_INSTRUCTIONS_CHAT_LOAD_SCOPE = "customInstructionsChatLoadScope";
43+
public static final String CUSTOM_INSTRUCTIONS_CHAT_LOAD_SCOPE_ALL = "allProjects";
44+
public static final String CUSTOM_INSTRUCTIONS_CHAT_LOAD_SCOPE_REFERENCED = "referencedProjects";
4245
public static final String GITHUB_COPILOT_URL = "http://github.com";
4346
@Deprecated
4447
public static final String QUICK_START_VERSION = "quickStartVersion";

‎com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/chat/CustomInstructionsChatLoadScope.java‎

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
// Copyright (c) Microsoft Corporation.
2+
// Licensed under the MIT license.
3+
4+
package com.microsoft.copilot.eclipse.core.chat;
5+
6+
import com.microsoft.copilot.eclipse.core.Constants;
7+
8+
/**
9+
* Scope loading modes for custom instructions in GitHub Copilot chat.
10+
* <ul>
11+
* <li><b>ALL_PROJECTS</b>: Load custom instructions from all projects in the Eclipse workspace
12+
* <li><b>REFERENCED_PROJECTS</b>: Load custom instructions only from parent projects of files/folders referenced in the
13+
* Copilot chat
14+
* </ul>
15+
*/
16+
public enum CustomInstructionsChatLoadScope {
17+
ALL_PROJECTS(Constants.CUSTOM_INSTRUCTIONS_CHAT_LOAD_SCOPE_ALL),
18+
REFERENCED_PROJECTS(Constants.CUSTOM_INSTRUCTIONS_CHAT_LOAD_SCOPE_REFERENCED);
19+
20+
public static final CustomInstructionsChatLoadScope DEFAULT_VALUE = CustomInstructionsChatLoadScope.ALL_PROJECTS;
21+
22+
private final String value;
23+
24+
CustomInstructionsChatLoadScope(String value) {
25+
this.value = value;
26+
}
27+
28+
/**
29+
* Returns the string value representing this enum entry for preference serialization.
30+
*
31+
* @return the string value for this preference setting
32+
*/
33+
public String getValue() {
34+
return value;
35+
}
36+
37+
/**
38+
* Retrieves the enum constant corresponding to the given string value if available, otherwise an
39+
* {@link IllegalArgumentException} is thrown.
40+
*
41+
* @param value the string value (preference) representing an enum entry
42+
* @return the enum entry representing the given value
43+
* @throws IllegalArgumentException if the value does not correspond to any enum entry
44+
*/
45+
public static CustomInstructionsChatLoadScope fromValue(String value) {
46+
for (CustomInstructionsChatLoadScope scope : values()) {
47+
if (scope.getValue().equals(value)) {
48+
return scope;
49+
}
50+
}
51+
throw new IllegalArgumentException("Unknown CustomInstructionsLoadScope value: " + value);
52+
}
53+
54+
}

‎com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/lsp/CopilotLanguageServerConnection.java‎

Lines changed: 22 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -281,17 +281,20 @@ public CompletableFuture<ChatCreateResult> createConversation(String workDoneTok
281281
String chatModeName, String customChatModeId, List<TodoItem> todos, String agentSlug,
282282
String agentJobWorkspaceFolder) {
283283
return createConversation(workDoneToken, message, files, currentFile, currentSelection, turns, activeModel,
284-
chatModeName, customChatModeId, todos, agentSlug, agentJobWorkspaceFolder, null, null);
284+
chatModeName, customChatModeId, todos, agentSlug, agentJobWorkspaceFolder, null, null,
285+
LSPEclipseUtils.getWorkspaceFolders());
285286
}
286287

287288
/**
288-
* Create a conversation with the given parameters, including optional conversationId and restoreToTurnId for session
289-
* restoration.
289+
* Create a conversation with the given parameters, including optional conversationId, restoreToTurnId for session
290+
* restoration, and optional workspace folders argument.
290291
*/
291292
public CompletableFuture<ChatCreateResult> createConversation(String workDoneToken, String message,
292293
List<IResource> files, IFile currentFile, Range currentSelection, List<Turn> turns, CopilotModel activeModel,
293294
String chatModeName, String customChatModeId, List<TodoItem> todos, String agentSlug,
294-
String agentJobWorkspaceFolder, String conversationId, String restoreToTurnId) {
295+
String agentJobWorkspaceFolder, String conversationId, String restoreToTurnId,
296+
List<WorkspaceFolder> workspaceFolders) {
297+
295298
boolean supportVision = activeModel.getCapabilities().supports().vision();
296299
Either<String, List<ChatCompletionContentPart>> messageWithImages = ChatMessageUtils
297300
.createMessageWithImages(message, FileUtils.filterFilesFrom(files), supportVision);
@@ -310,7 +313,7 @@ public CompletableFuture<ChatCreateResult> createConversation(String workDoneTok
310313

311314
if (StringUtils.isBlank(agentSlug)) {
312315
param.setWorkspaceFolder(PlatformUtils.getWorkspaceRootUri());
313-
param.setWorkspaceFolders(LSPEclipseUtils.getWorkspaceFolders());
316+
param.setWorkspaceFolders(workspaceFolders == null ? List.of() : workspaceFolders);
314317
param.setTodoList(todos);
315318
} else {
316319
// Set agentSlug on the last turn (current user message) after history insertion
@@ -357,6 +360,19 @@ public CompletableFuture<ChatTurnResult> addConversationTurn(String workDoneToke
357360
String message, List<IResource> files, IFile currentFile, Range currentSelection, CopilotModel activeModel,
358361
String chatModeName, String customChatModeId, List<TodoItem> todoList, String agentSlug,
359362
String agentJobWorkspaceFolder) {
363+
return addConversationTurn(workDoneToken, conversationId, message, files, currentFile, currentSelection,
364+
activeModel, chatModeName, customChatModeId, todoList, agentSlug, agentJobWorkspaceFolder,
365+
LSPEclipseUtils.getWorkspaceFolders());
366+
}
367+
368+
/**
369+
* Create a conversation turn with the given parameters, including optional workspace folders argument.
370+
*/
371+
public CompletableFuture<ChatTurnResult> addConversationTurn(String workDoneToken, String conversationId,
372+
String message, List<IResource> files, IFile currentFile, Range currentSelection, CopilotModel activeModel,
373+
String chatModeName, String customChatModeId, List<TodoItem> todoList, String agentSlug,
374+
String agentJobWorkspaceFolder, List<WorkspaceFolder> workspaceFolders) {
375+
360376
boolean supportVision = activeModel.getCapabilities().supports().vision();
361377
Either<String, List<ChatCompletionContentPart>> messageWithImages = ChatMessageUtils
362378
.createMessageWithImages(message, FileUtils.filterFilesFrom(files), supportVision);
@@ -370,7 +386,7 @@ public CompletableFuture<ChatTurnResult> addConversationTurn(String workDoneToke
370386

371387
if (StringUtils.isBlank(agentSlug)) {
372388
param.setWorkspaceFolder(PlatformUtils.getWorkspaceRootUri());
373-
param.setWorkspaceFolders(LSPEclipseUtils.getWorkspaceFolders());
389+
param.setWorkspaceFolders(workspaceFolders == null ? List.of() : workspaceFolders);
374390
param.setTodoList(todoList);
375391
} else {
376392
param.setAgentSlug(agentSlug);

‎com.microsoft.copilot.eclipse.ui.test/META-INF/MANIFEST.MF‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ Require-Bundle: org.mockito.mockito-core;bundle-version="5.14.2",
1212
org.eclipse.lsp4e;bundle-version="0.18.1",
1313
org.eclipse.jdt.annotation;resolution:=optional,
1414
junit-jupiter-api;bundle-version="5.10.1",
15+
junit-jupiter-params;bundle-version="5.10.1",
1516
org.mockito.junit-jupiter;bundle-version="5.10.2",
1617
com.microsoft.copilot.eclipse.core;bundle-version="0.15.0",
1718
com.microsoft.copilot.eclipse.ui;bundle-version="0.15.0",

‎com.microsoft.copilot.eclipse.ui.test/src/com/microsoft/copilot/eclipse/ui/utils/PreferencesUtilsTest.java‎

Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,89 @@
1+
// Copyright (c) Microsoft Corporation.
2+
// Licensed under the MIT license.
3+
4+
package com.microsoft.copilot.eclipse.ui.utils;
5+
6+
import static org.junit.jupiter.api.Assertions.assertEquals;
7+
8+
import org.eclipse.jface.preference.IPreferenceStore;
9+
import org.eclipse.jface.preference.PreferenceStore;
10+
import org.junit.jupiter.api.AfterEach;
11+
import org.junit.jupiter.api.BeforeEach;
12+
import org.junit.jupiter.api.Test;
13+
import org.junit.jupiter.params.ParameterizedTest;
14+
import org.junit.jupiter.params.provider.EnumSource;
15+
16+
import com.microsoft.copilot.eclipse.core.Constants;
17+
import com.microsoft.copilot.eclipse.core.chat.CustomInstructionsChatLoadScope;
18+
19+
class PreferencesUtilsTest {
20+
21+
private IPreferenceStore store;
22+
23+
@BeforeEach
24+
void setUp() {
25+
store = new PreferenceStore();
26+
store.setDefault(Constants.CUSTOM_INSTRUCTIONS_CHAT_LOAD_SCOPE,
27+
CustomInstructionsChatLoadScope.DEFAULT_VALUE.getValue());
28+
}
29+
30+
@AfterEach
31+
void tearDown() {
32+
store = null;
33+
}
34+
35+
@ParameterizedTest
36+
@EnumSource(CustomInstructionsChatLoadScope.class)
37+
void testGetCustomInstructionsChatLoadScope_returnsStoredValue(CustomInstructionsChatLoadScope scope) {
38+
store.setValue(Constants.CUSTOM_INSTRUCTIONS_CHAT_LOAD_SCOPE, scope.getValue());
39+
40+
CustomInstructionsChatLoadScope result = PreferencesUtils.getCustomInstructionsChatLoadScope(store);
41+
42+
assertEquals(scope, result);
43+
}
44+
45+
@Test
46+
void testGetCustomInstructionsChatLoadScope_fallsBackToDefaultInCaseOfInvalidValueInStore() {
47+
store.setValue(Constants.CUSTOM_INSTRUCTIONS_CHAT_LOAD_SCOPE, "invalid_value");
48+
49+
CustomInstructionsChatLoadScope result = PreferencesUtils.getCustomInstructionsChatLoadScope(store);
50+
51+
assertEquals(CustomInstructionsChatLoadScope.DEFAULT_VALUE, result);
52+
assertEquals(CustomInstructionsChatLoadScope.DEFAULT_VALUE.getValue(),
53+
store.getString(Constants.CUSTOM_INSTRUCTIONS_CHAT_LOAD_SCOPE),
54+
"The invalid stored value should have been corrected to the default");
55+
}
56+
57+
@Test
58+
void testGetCustomInstructionsChatLoadScopeDefault_returnsConfiguredDefault() {
59+
// explicitly store varying values for InstanceScope and DefaultScope
60+
store.setValue(Constants.CUSTOM_INSTRUCTIONS_CHAT_LOAD_SCOPE,
61+
CustomInstructionsChatLoadScope.ALL_PROJECTS.getValue());
62+
store.setDefault(Constants.CUSTOM_INSTRUCTIONS_CHAT_LOAD_SCOPE,
63+
CustomInstructionsChatLoadScope.REFERENCED_PROJECTS.getValue());
64+
65+
CustomInstructionsChatLoadScope result = PreferencesUtils.getCustomInstructionsChatLoadScopeDefault(store);
66+
67+
// Should return the default value, not the stored value
68+
assertEquals(CustomInstructionsChatLoadScope.REFERENCED_PROJECTS, result);
69+
}
70+
71+
@Test
72+
void testGetCustomInstructionsChatLoadScopeDefault_fallsBackToValidDefaultInCaseOfInvalidValue() {
73+
String invalidDefaultValue = "invalid_default_value";
74+
String instanceScopValue = CustomInstructionsChatLoadScope.REFERENCED_PROJECTS.getValue();
75+
store.setDefault(Constants.CUSTOM_INSTRUCTIONS_CHAT_LOAD_SCOPE, invalidDefaultValue);
76+
store.setValue(Constants.CUSTOM_INSTRUCTIONS_CHAT_LOAD_SCOPE, instanceScopValue);
77+
78+
CustomInstructionsChatLoadScope result = PreferencesUtils.getCustomInstructionsChatLoadScopeDefault(store);
79+
80+
assertEquals(CustomInstructionsChatLoadScope.DEFAULT_VALUE, result);
81+
assertEquals(invalidDefaultValue,
82+
store.getDefaultString(Constants.CUSTOM_INSTRUCTIONS_CHAT_LOAD_SCOPE),
83+
"The stored DefaultScope value should not change, even if it's invalid. "
84+
+ "That is repsonsibility of a PreferenceInitializer.");
85+
assertEquals(instanceScopValue,
86+
store.getString(Constants.CUSTOM_INSTRUCTIONS_CHAT_LOAD_SCOPE),
87+
"The stored InstanceScope value should not change");
88+
}
89+
}

‎com.microsoft.copilot.eclipse.ui.test/src/com/microsoft/copilot/eclipse/ui/utils/ResourceUtilsTest.java‎

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,17 +7,25 @@
77
import static org.mockito.Mockito.*;
88

99
import java.util.Arrays;
10+
import java.util.List;
11+
import java.util.stream.Stream;
1012

1113
import org.eclipse.core.resources.IFile;
1214
import org.eclipse.core.resources.IFolder;
15+
import org.eclipse.core.resources.IProject;
16+
import org.eclipse.core.resources.IResource;
1317
import org.eclipse.jface.viewers.StructuredSelection;
18+
import org.eclipse.lsp4e.LSPEclipseUtils;
19+
import org.eclipse.lsp4j.WorkspaceFolder;
1420
import org.junit.jupiter.api.AfterEach;
1521
import org.junit.jupiter.api.BeforeEach;
1622
import org.junit.jupiter.api.Test;
1723
import org.junit.jupiter.api.extension.ExtendWith;
1824
import org.mockito.Mock;
1925
import org.mockito.MockedStatic;
2026
import org.mockito.junit.jupiter.MockitoExtension;
27+
import org.junit.jupiter.params.ParameterizedTest;
28+
import org.junit.jupiter.params.provider.MethodSource;
2129

2230
import com.microsoft.copilot.eclipse.core.utils.FileUtils;
2331

@@ -35,21 +43,40 @@ class ResourceUtilsTest {
3543

3644
@Mock
3745
private IFolder mockFolder;
46+
47+
@Mock
48+
private IProject mockProjectA;
49+
50+
@Mock
51+
private IProject mockProjectB;
3852

3953
private MockedStatic<FileUtils> mockedFileUtils;
54+
private MockedStatic<LSPEclipseUtils> mockedLspUtils;
55+
56+
private static final String nameProjectA= "ProjectA";
57+
private static final String nameProjectB= "ProjectB";
4058

4159
@BeforeEach
4260
void setUp() {
4361
mockedFileUtils = mockStatic(FileUtils.class);
4462
mockedFileUtils.when(() -> FileUtils.isExcludedFromReferencedFiles(mockValidFile)).thenReturn(false);
4563
mockedFileUtils.when(() -> FileUtils.isExcludedFromReferencedFiles(mockInvalidFile)).thenReturn(true);
64+
65+
mockedLspUtils = mockStatic(LSPEclipseUtils.class);
66+
mockedLspUtils.when(() -> LSPEclipseUtils.toWorkspaceFolder(mockProjectA))
67+
.thenReturn(new WorkspaceFolder("file:///" + nameProjectA, nameProjectA));
68+
mockedLspUtils.when(() -> LSPEclipseUtils.toWorkspaceFolder(mockProjectB))
69+
.thenReturn(new WorkspaceFolder("file:///" + nameProjectB, nameProjectB));
4670
}
4771

4872
@AfterEach
4973
void tearDown() {
5074
if (mockedFileUtils != null) {
5175
mockedFileUtils.close();
5276
}
77+
if (mockedLspUtils != null) {
78+
mockedLspUtils.close();
79+
}
5380
}
5481

5582
@Test
@@ -82,4 +109,52 @@ void testCollectValidResourcesWithMocks() {
82109
assertTrue(validResources.contains(mockFolder), "Should contain folder");
83110
assertFalse(validResources.contains(mockInvalidFile), "Should not contain excluded file");
84111
}
112+
113+
private static Stream<List<IResource>> provideResourcesForNeverNullTest() {
114+
return Stream.of(
115+
null,
116+
List.of(),
117+
Arrays.asList((IResource) null),
118+
List.of(mock(IFolder.class)),
119+
List.of(mock(IProject.class))
120+
);
121+
}
122+
123+
@ParameterizedTest
124+
@MethodSource("provideResourcesForNeverNullTest")
125+
void testDeriveWorkspaceFoldersReturnsNeverNull(List<IResource> resources) {
126+
List<WorkspaceFolder> result = ResourceUtils.deriveWorkspaceFoldersFrom(resources);
127+
128+
assertNotNull(result);
129+
assertTrue(result.isEmpty());
130+
}
131+
132+
@Test
133+
void testDeriveWorkspaceFoldersWithMultipleResources() {
134+
when(mockValidFile.getProject()).thenReturn(mockProjectA);
135+
when(mockFolder.getProject()).thenReturn(mockProjectB);
136+
when(mockProjectA.isAccessible()).thenReturn(true);
137+
when(mockProjectB.isAccessible()).thenReturn(true);
138+
139+
List<WorkspaceFolder> result = ResourceUtils.deriveWorkspaceFoldersFrom(List.of(mockValidFile, mockFolder));
140+
141+
assertNotNull(result);
142+
assertFalse(result.isEmpty());
143+
assertEquals(2, result.size(), "Both projects from both resources should be derived as workspace folders");
144+
}
145+
146+
@Test
147+
void testDeriveWorkspaceFoldersDoesNotReturnDuplicates() {
148+
when(mockValidFile.getProject()).thenReturn(mockProjectA);
149+
when(mockFolder.getProject()).thenReturn(mockProjectA);
150+
when(mockProjectA.isAccessible()).thenReturn(true);
151+
when(mockProjectA.getName()).thenReturn(nameProjectA);
152+
153+
List<WorkspaceFolder> result = ResourceUtils.deriveWorkspaceFoldersFrom(List.of(mockValidFile, mockFolder));
154+
155+
assertNotNull(result);
156+
assertFalse(result.isEmpty());
157+
assertEquals(1, result.size(), "Projects in derived workspaces folders should be unique, no duplicates.");
158+
assertEquals(mockProjectA.getName(), result.get(0).getName());
159+
}
85160
}

0 commit comments

Comments
 (0)