Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
2 changes: 1 addition & 1 deletion src/extension/byok/common/geminiMessageConverter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ function apiContentToGeminiContent(content: (LanguageModelTextPart | LanguageMod
// Handle case with text content (may also have images)
try {
responsePayload = JSON.parse(textContent);
Copy link

Copilot AI Jan 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider adding a comment explaining why arrays (and other primitive types) need to be wrapped in an object. This would help future maintainers understand the Gemini API requirement. For example:

// Wrap non-object values (arrays, primitives, null) in an object because
// Gemini's FunctionResponse expects response to be a google.protobuf.Struct (JSON Object)

This provides context about the API constraint that necessitates this wrapping behavior.

Suggested change
responsePayload = JSON.parse(textContent);
responsePayload = JSON.parse(textContent);
// Wrap non-object values (arrays, primitives, null) in an object because
// Gemini's FunctionResponse.response must be a google.protobuf.Struct (JSON object)

Copilot uses AI. Check for mistakes.
if (typeof responsePayload !== 'object' || responsePayload === null) {
if (typeof responsePayload !== 'object' || responsePayload === null || Array.isArray(responsePayload)) {
responsePayload = { result: responsePayload };
}
} catch {
Expand Down
18 changes: 18 additions & 0 deletions src/extension/byok/common/test/geminiMessageConverter.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,24 @@ describe('GeminiMessageConverter', () => {
expect(fr.functionResponse.response).toEqual({ foo: 'bar' });
});

it('should wrap array responses in an object', () => {
const toolResult = new LanguageModelToolResultPart('listRepos_12345', [new LanguageModelTextPart('["repo1", "repo2"]')]);
const messages: LanguageModelChatMessage[] = [
{
role: LanguageModelChatMessageRole.Assistant,
content: [toolResult],
name: undefined
}
];

const result = apiMessageToGeminiMessage(messages);

expect(result.contents).toHaveLength(1);
expect(result.contents[0].role).toBe('user');
const fr: any = result.contents[0].parts![0];
expect(fr.functionResponse.response).toEqual({ result: ['repo1', 'repo2'] });
});
Comment on lines +105 to +121
Copy link

Copilot AI Jan 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While the test for array wrapping is good, the fix also wraps other primitive types (strings, numbers, booleans, null) in an object. Consider adding tests for these cases as well to ensure comprehensive coverage. For example:

  • Test with a string primitive: "simple string"
  • Test with a number: 42
  • Test with a boolean: true
  • Test with null: null

This would ensure that all code paths through the wrapping logic are tested.

Copilot uses AI. Check for mistakes.

it('should be idempotent when called multiple times (no duplication)', () => {
const toolResult = new LanguageModelToolResultPart('doThing_12345', [new LMText('{"value":42}')]);
const messages: LanguageModelChatMessage[] = [
Expand Down