feat(core,cli): add support for SGLang and local OpenAI-compatible en… - #28681
feat(core,cli): add support for SGLang and local OpenAI-compatible en…#28681shivajid wants to merge 12 commits into
Conversation
|
📊 PR Size: size/XL
|
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request expands the core and CLI capabilities by introducing support for SGLang and OpenAI-compatible API endpoints. It includes a new content generator implementation, updates to the authentication detection mechanism, and adds support for the Kimi-K3 model, allowing users to leverage local or remote model servers beyond the existing Google-native options. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request introduces support for SGLang servers (local or remote Kimi-K3) by adding a new SGLANG authentication type, updating environment variable detection, and implementing the SglangContentGenerator class. The code review identified a critical issue where generated tool_call_ids are not mapped to tool responses, which would lead to API errors. Additionally, the reviewer recommended refactoring generateContentStream to return Promise to match the ContentGenerator interface, allowing the removal of an unsafe 'as never' type assertion in contentGenerator.ts.
| private convertContentsToMessages( | ||
| contents: Content[] | string | undefined, | ||
| systemInstruction?: Content | string, | ||
| ) { | ||
| const messages: Array<{ | ||
| role: string; | ||
| content?: string | null; | ||
| tool_calls?: Array<{ | ||
| id: string; | ||
| type: 'function'; | ||
| function: { name: string; arguments: string }; | ||
| }>; | ||
| tool_call_id?: string; | ||
| }> = []; | ||
|
|
||
| if (systemInstruction) { | ||
| const text = | ||
| typeof systemInstruction === 'string' | ||
| ? systemInstruction | ||
| : systemInstruction.parts | ||
| ?.map((p: Part) => p.text) | ||
| .filter(Boolean) | ||
| .join('\n') || ''; | ||
| if (text) { | ||
| messages.push({ role: 'system', content: text }); | ||
| } | ||
| } | ||
|
|
||
| if (typeof contents === 'string') { | ||
| messages.push({ role: 'user', content: contents }); | ||
| return messages; | ||
| } | ||
|
|
||
| if (Array.isArray(contents)) { | ||
| for (const c of contents) { | ||
| if (!c || !c.parts) continue; | ||
| const role = c.role === 'model' ? 'assistant' : 'user'; | ||
|
|
||
| let textParts = ''; | ||
| const toolCalls: Array<{ | ||
| id: string; | ||
| type: 'function'; | ||
| function: { name: string; arguments: string }; | ||
| }> = []; | ||
|
|
||
| for (const part of c.parts) { | ||
| if (part.text) { | ||
| textParts += (textParts ? '\n' : '') + part.text; | ||
| } | ||
| if (part.functionCall) { | ||
| toolCalls.push({ | ||
| id: `call_${Math.random().toString(36).substring(2, 9)}`, | ||
| type: 'function', | ||
| function: { | ||
| name: part.functionCall.name, | ||
| arguments: JSON.stringify(part.functionCall.args || {}), | ||
| }, | ||
| }); | ||
| } | ||
| if (part.functionResponse) { | ||
| messages.push({ | ||
| role: 'tool', | ||
| content: JSON.stringify(part.functionResponse.response || {}), | ||
| tool_call_id: | ||
| (part.functionResponse as { id?: string }).id || | ||
| `call_${part.functionResponse.name}`, | ||
| }); | ||
| } | ||
| } | ||
|
|
||
| if (toolCalls.length > 0) { | ||
| messages.push({ | ||
| role: 'assistant', | ||
| content: textParts || null, | ||
| tool_calls: toolCalls, | ||
| }); | ||
| } else if (textParts) { | ||
| messages.push({ role, content: textParts }); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| return messages; | ||
| } |
There was a problem hiding this comment.
The current implementation of convertContentsToMessages generates a random tool_call_id for assistant tool calls, but does not map them to the corresponding tool responses. This will cause a mismatch between the assistant's tool_calls and the tool responses, leading to API errors (400 Bad Request) from SGLang/OpenAI-compatible endpoints.
We should maintain a map of function names to generated tool call IDs during conversion to ensure they match perfectly.
private convertContentsToMessages(
contents: Content[] | string | undefined,
systemInstruction?: Content | string,
) {
const messages: Array<{
role: string;
content?: string | null;
tool_calls?: Array<{
id: string;
type: 'function';
function: { name: string; arguments: string };
}>;
tool_call_id?: string;
}> = [];
const toolCallIds = new Map<string, string>();
if (systemInstruction) {
const text =
typeof systemInstruction === 'string'
? systemInstruction
: systemInstruction.parts
?.map((p: Part) => p.text)
.filter(Boolean)
.join('\n') || '';
if (text) {
messages.push({ role: 'system', content: text });
}
}
if (typeof contents === 'string') {
messages.push({ role: 'user', content: contents });
return messages;
}
if (Array.isArray(contents)) {
for (const c of contents) {
if (!c || !c.parts) continue;
const role = c.role === 'model' ? 'assistant' : 'user';
let textParts = '';
const toolCalls: Array<{
id: string;
type: 'function';
function: { name: string; arguments: string };
}> = [];
for (const part of c.parts) {
if (part.text) {
textParts += (textParts ? '\n' : '') + part.text;
}
if (part.functionCall) {
const callId = `call_${Math.random().toString(36).substring(2, 9)}`;
toolCallIds.set(part.functionCall.name, callId);
toolCalls.push({
id: callId,
type: 'function',
function: {
name: part.functionCall.name,
arguments: JSON.stringify(part.functionCall.args || {}),
},
});
}
if (part.functionResponse) {
const callId =
(part.functionResponse as { id?: string }).id ||
toolCallIds.get(part.functionResponse.name) ||
`call_${part.functionResponse.name}`;
messages.push({
role: 'tool',
content: JSON.stringify(part.functionResponse.response || {}),
tool_call_id: callId,
});
}
}
if (toolCalls.length > 0) {
messages.push({
role: 'assistant',
content: textParts || null,
tool_calls: toolCalls,
});
} else if (textParts) {
messages.push({ role, content: textParts });
}
}
}
return messages;
}| async *generateContentStream( | ||
| request: GenerateContentParameters, | ||
| _userPromptId: string, | ||
| _role: LlmRole, | ||
| ): AsyncGenerator<GenerateContentResponse> { | ||
| const messages = this.convertContentsToMessages( | ||
| request.contents as Content[] | string, | ||
| request.config?.systemInstruction as Content | string, | ||
| ); | ||
| const tools = this.convertTools(request.config); | ||
|
|
||
| const payload: Record<string, unknown> = { | ||
| model: request.model || this.defaultModel, | ||
| messages, | ||
| temperature: request.config?.temperature ?? 0.7, | ||
| max_tokens: request.config?.maxOutputTokens ?? 4096, | ||
| stream: true, | ||
| }; | ||
| if (tools) { | ||
| payload['tools'] = tools; | ||
| } | ||
|
|
||
| const res = await fetch(`${this.baseUrl}/chat/completions`, { | ||
| method: 'POST', | ||
| headers: { 'Content-Type': 'application/json' }, | ||
| body: JSON.stringify(payload), | ||
| }); | ||
|
|
||
| if (!res.ok || !res.body) { | ||
| throw new Error(`SGLang stream error (${res.status}): ${await res.text()}`); | ||
| } | ||
|
|
||
| const reader = res.body.getReader(); | ||
| const decoder = new TextDecoder(); | ||
| let buffer = ''; | ||
|
|
||
| while (true) { | ||
| const { done, value } = await reader.read(); | ||
| if (done) break; | ||
| buffer += decoder.decode(value, { stream: true }); | ||
| const lines = buffer.split('\n'); | ||
| buffer = lines.pop() || ''; | ||
|
|
||
| for (const line of lines) { | ||
| const trimmed = line.trim(); | ||
| if (!trimmed || !trimmed.startsWith('data: ')) continue; | ||
| if (trimmed === 'data: [DONE]') return; | ||
| try { | ||
| const json = JSON.parse(trimmed.slice(6)); | ||
| const delta = json.choices?.[0]?.delta; | ||
| if (delta?.content) { | ||
| yield { | ||
| candidates: [ | ||
| { | ||
| content: { parts: [{ text: delta.content }], role: 'model' }, | ||
| }, | ||
| ], | ||
| } as GenerateContentResponse; | ||
| } | ||
| } catch { | ||
| // ignore partial chunks | ||
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
The ContentGenerator interface defines generateContentStream as returning Promise<AsyncGenerator<GenerateContentResponse>>. By implementing it as an async * function (which returns AsyncGenerator), there is a type mismatch requiring an unsafe as never cast in contentGenerator.ts.
Additionally, using async * defers the fetch call and res.ok check until the caller starts iterating over the generator. Changing this to return Promise<AsyncGenerator> ensures connection/HTTP errors are thrown immediately when the stream is initiated.
async generateContentStream(
request: GenerateContentParameters,
_userPromptId: string,
_role: LlmRole,
): Promise<AsyncGenerator<GenerateContentResponse>> {
const messages = this.convertContentsToMessages(
request.contents as Content[] | string,
request.config?.systemInstruction as Content | string,
);
const tools = this.convertTools(request.config);
const payload: Record<string, unknown> = {
model: request.model || this.defaultModel,
messages,
temperature: request.config?.temperature,
max_tokens: request.config?.maxOutputTokens,
stream: true,
};
if (tools) {
payload['tools'] = tools;
}
const res = await fetch(`${this.baseUrl}/chat/completions`, {
method: 'POST',
headers: { 'Content-Type': 'application/json' },
body: JSON.stringify(payload),
});
if (!res.ok || !res.body) {
throw new Error(`SGLang stream error (${res.status}): ${await res.text()}`);
}
const reader = res.body.getReader();
const decoder = new TextDecoder();
async function* generator() {
let buffer = '';
try {
while (true) {
const { done, value } = await reader.read();
if (done) break;
buffer += decoder.decode(value, { stream: true });
const lines = buffer.split('\n');
buffer = lines.pop() || '';
for (const line of lines) {
const trimmed = line.trim();
if (!trimmed || !trimmed.startsWith('data: ')) continue;
if (trimmed === 'data: [DONE]') return;
try {
const json = JSON.parse(trimmed.slice(6));
const delta = json.choices?.[0]?.delta;
if (delta?.content) {
yield {
candidates: [
{
content: { parts: [{ text: delta.content }], role: 'model' },
},
],
} as GenerateContentResponse;
}
} catch {
// ignore partial chunks
}
}
}
} finally {
reader.releaseLock();
}
}
return generator();
}References
- Rely on the schema as the single source of truth for configuration defaults, avoiding redundant nullish coalescing operators.
| return new LoggingContentGenerator( | ||
| new SglangContentGenerator(baseUrl, modelName) as never, | ||
| gcConfig, | ||
| ); |
There was a problem hiding this comment.
With SglangContentGenerator properly implementing the ContentGenerator interface, we can remove the unsafe as never type assertion.
| return new LoggingContentGenerator( | |
| new SglangContentGenerator(baseUrl, modelName) as never, | |
| gcConfig, | |
| ); | |
| return new LoggingContentGenerator( | |
| new SglangContentGenerator(baseUrl, modelName), | |
| gcConfig, | |
| ); |
…cs in SglangContentGenerator
…nAI API compatibility
… Kimi-K3 - Return real GenerateContentResponse instances so SDK getters (functionCalls) used by turn.ts/geminiChat.ts dispatch tool calls - Convert parametersJsonSchema (used by all built-in tools), not just parameters, so tools are advertised with their schemas - Preserve tool call ids across turns (tool_calls[].id == tool_call_id) - Honor config.abortSignal so ESC cancels generation - Support responseJsonSchema/responseMimeType via response_format for internal generateJson calls (next-speaker, loop detection) - Stream usage via stream_options.include_usage; estimate countTokens - Map gemini-* model aliases to the served model name - Stable **Thinking** thought subject; never echo reasoning to server - Remove console.error calls that corrupt the Ink UI; raise max_tokens default to 32768 so large edit/write tool args aren't truncated
|
Hi there! Thank you for your interest in contributing to Gemini CLI. To ensure we maintain high code quality and focus on our prioritized roadmap, we only guarantee review and consideration of pull requests for issues that are explicitly labeled as 'help wanted'. This PR will be closed in 7 days if it remains without that designation. We encourage you to find and contribute to existing 'help wanted' issues in our backlog! Thank you for your understanding. |
|
This pull request is being closed as it has been open for 14 days without a 'help wanted' designation. We encourage you to find and contribute to existing 'help wanted' issues in our backlog! Thank you for your understanding. |
…dpoints
Summary
Details
Related Issues
How to Validate
Pre-Merge Checklist