Skip to content

Commit 949de90

Browse files
rinceyuanj-zhangyiyuanedburns
authored
java: enforce non-blank @CopilotToolParam description at compile time (#1980)
Closes #1836 Add compile-time validation in CopilotToolProcessor to reject @CopilotToolParam annotations with blank value (description). - Blank or whitespace-only descriptions now produce a compile error - Single-record wrapper parameters are exempt (they delegate to record component annotations) - Error message identifies parameter name, method, and class - 5 new tests covering blank, whitespace, valid, unannotated, and record-wrapper cases Co-authored-by: j-zhangyiyuan <j-zhangyiyuan@microsoft.com> Co-authored-by: Ed Burns <edburns@microsoft.com>
1 parent db0edd8 commit 949de90

2 files changed

Lines changed: 118 additions & 0 deletions

File tree

java/src/main/java/com/github/copilot/tool/CopilotToolProcessor.java

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -127,6 +127,24 @@ public boolean process(Set<? extends TypeElement> annotations, RoundEnvironment
127127
}
128128
}
129129
}
130+
131+
// Validate blank @CopilotToolParam descriptions (exempt single-record wrappers)
132+
boolean isSingleRecordWrapper = schemaParameters.size() == 1 && isRecord(schemaParameters.get(0).asType());
133+
for (VariableElement param : schemaParameters) {
134+
if (isSingleRecordWrapper && param.equals(schemaParameters.get(0))) {
135+
continue;
136+
}
137+
CopilotToolParam paramAnnotation = param.getAnnotation(CopilotToolParam.class);
138+
if (paramAnnotation != null && paramAnnotation.value().isBlank()) {
139+
TypeElement enclosingClass = (TypeElement) method.getEnclosingElement();
140+
processingEnv.getMessager().printMessage(Diagnostic.Kind.ERROR,
141+
"@CopilotToolParam on parameter '" + param.getSimpleName() + "' in '"
142+
+ enclosingClass.getSimpleName() + "." + method.getSimpleName()
143+
+ "' has a blank value (description). "
144+
+ "Descriptions are required so the LLM can correctly select and invoke the tool",
145+
param);
146+
}
147+
}
130148
}
131149

132150
// Group methods by enclosing type

java/src/test/java/com/github/copilot/tool/CopilotToolProcessorTest.java

Lines changed: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -189,6 +189,106 @@ public String search(@CopilotToolParam(value = "Search input", required = false,
189189
"Expected compile error for single-record wrapper metadata overrides, got: " + result.diagnostics);
190190
}
191191

192+
// ── Test: Blank @CopilotToolParam description validation ────────────────────
193+
194+
@Test
195+
void emitsError_forBlankParamDescription() {
196+
String source = """
197+
package test;
198+
import com.github.copilot.tool.CopilotTool;
199+
import com.github.copilot.tool.CopilotToolParam;
200+
public class BlankDescTools {
201+
@CopilotTool("Search for items")
202+
public String searchItems(@CopilotToolParam("") String query) {
203+
return "results for " + query;
204+
}
205+
}
206+
""";
207+
208+
CompilationResult result = compileWithProcessor(List.of(inMemorySource("test.BlankDescTools", source)));
209+
210+
assertTrue(hasErrorContaining(result, "blank value (description)"),
211+
"Expected compile error for blank @CopilotToolParam description, got: " + result.diagnostics);
212+
}
213+
214+
@Test
215+
void emitsError_forWhitespaceOnlyParamDescription() {
216+
String source = """
217+
package test;
218+
import com.github.copilot.tool.CopilotTool;
219+
import com.github.copilot.tool.CopilotToolParam;
220+
public class WhitespaceDescTools {
221+
@CopilotTool("Search for items")
222+
public String searchItems(@CopilotToolParam(" ") String query) {
223+
return "results for " + query;
224+
}
225+
}
226+
""";
227+
228+
CompilationResult result = compileWithProcessor(List.of(inMemorySource("test.WhitespaceDescTools", source)));
229+
230+
assertTrue(hasErrorContaining(result, "blank value (description)"),
231+
"Expected compile error for whitespace-only @CopilotToolParam description, got: " + result.diagnostics);
232+
}
233+
234+
@Test
235+
void compilesSuccessfully_forValidParamDescription() {
236+
String source = """
237+
package test;
238+
import com.github.copilot.tool.CopilotTool;
239+
import com.github.copilot.tool.CopilotToolParam;
240+
public class ValidDescTools {
241+
@CopilotTool("Search for items")
242+
public String searchItems(@CopilotToolParam("Search query") String query) {
243+
return "results for " + query;
244+
}
245+
}
246+
""";
247+
248+
CompilationResult result = compileWithProcessor(List.of(inMemorySource("test.ValidDescTools", source)));
249+
250+
assertNoErrors(result);
251+
}
252+
253+
@Test
254+
void compilesSuccessfully_forParamWithoutAnnotation() {
255+
String source = """
256+
package test;
257+
import com.github.copilot.tool.CopilotTool;
258+
public class NoAnnotationTools {
259+
@CopilotTool("Search for items")
260+
public String searchItems(String query) {
261+
return "results for " + query;
262+
}
263+
}
264+
""";
265+
266+
CompilationResult result = compileWithProcessor(List.of(inMemorySource("test.NoAnnotationTools", source)));
267+
268+
assertNoErrors(result);
269+
}
270+
271+
@Test
272+
void doesNotEmitBlankError_forSingleRecordWrapperWithDefaultAnnotation() {
273+
String source = """
274+
package test;
275+
import com.github.copilot.tool.CopilotTool;
276+
import com.github.copilot.tool.CopilotToolParam;
277+
public class RecordWrapperTools {
278+
public record SearchArgs(String query, int limit) {}
279+
@CopilotTool("Search for items")
280+
public String search(@CopilotToolParam SearchArgs args) {
281+
return args.query();
282+
}
283+
}
284+
""";
285+
286+
CompilationResult result = compileWithProcessor(List.of(inMemorySource("test.RecordWrapperTools", source)));
287+
288+
assertFalse(hasErrorContaining(result, "blank value (description)"),
289+
"Single-record wrapper should be exempt from blank description check, got: " + result.diagnostics);
290+
}
291+
192292
// ── Test: Return type handling ──────────────────────────────────────────────
193293

194294
@Test

0 commit comments

Comments
 (0)