Skip to content

Support OpenAI reasoning models with intelligent pattern detection #882

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.

Sign up for GitHub

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

Open
Copilot wants to merge 14 commits into
base: master
Choose a base branch
from
Open

Support OpenAI reasoning models with intelligent pattern detection #882

Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
3c6f522
Initial plan
Copilot Aug 23, 2025
2be919e
Add OpenAI o1 series model support with API compatibility
Copilot Aug 23, 2025
f4ac0ae
Extend reasoning model support and fix error handling in API responses
Copilot Aug 23, 2025
17b9876
Replace isUsingO1Model with isUsingReasoningModel and remove o1/o3 fr…
Copilot Aug 23, 2025
9062561
Remove old model definitions (o1, o3 series) from codebase
Copilot Aug 23, 2025
f4b3fd9
Merge branch 'master' into copilot/fix-792
PeterDaveHello Aug 23, 2025
79d378e
Implement smarter reasoning model detection using regex patterns
Copilot Aug 23, 2025
65ec6b2
Update src/services/apis/openai-api.mjs
PeterDaveHello Aug 23, 2025
712eb8e
Address review comments: fix system message filtering, improve error …
Copilot Aug 23, 2025
520c45c
Update src/services/apis/openai-api.mjs
PeterDaveHello Aug 29, 2025
08b7cca
Fix syntax error in OpenAI API file to resolve CI build failure
Copilot Aug 29, 2025
a3bf9a5
Address Copilot and Codex bot feedback: fix regex pattern and enhance…
Copilot Aug 29, 2025
f878898
Improve reasoning model detection and content processing
Copilot Aug 30, 2025
96cbd20
Address automated code review feedback: improve error handling, valid…
Copilot Aug 30, 2025
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
13 changes: 13 additions & 0 deletions src/config/index.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,10 @@ export const chatgptApiModelKeys = [
'chatgptApi4_1',
'chatgptApi4_1_mini',
'chatgptApi4_1_nano',
'chatgptApiO4Mini',
'chatgptApiGpt5',
'chatgptApiGpt5Mini',
'chatgptApiGpt5Nano',
]
export const customApiModelKeys = ['customModel']
export const ollamaApiModelKeys = ['ollamaModel']
Expand Down Expand Up @@ -256,6 +260,11 @@ export const Models = {
chatgptApi4_1_mini: { value: 'gpt-4.1-mini', desc: 'ChatGPT (GPT-4.1 mini)' },
chatgptApi4_1_nano: { value: 'gpt-4.1-nano', desc: 'ChatGPT (GPT-4.1 nano)' },

chatgptApiO4Mini: { value: 'o4-mini', desc: 'ChatGPT (o4-mini)' },
chatgptApiGpt5: { value: 'gpt-5', desc: 'ChatGPT (gpt-5)' },
chatgptApiGpt5Mini: { value: 'gpt-5-mini', desc: 'ChatGPT (gpt-5-mini)' },
chatgptApiGpt5Nano: { value: 'gpt-5-nano', desc: 'ChatGPT (gpt-5-nano)' },

claude2WebFree: { value: '', desc: 'Claude.ai (Web)' },
claude12Api: { value: 'claude-instant-1.2', desc: 'Claude.ai (API, Claude Instant 1.2)' },
claude2Api: { value: 'claude-2.0', desc: 'Claude.ai (API, Claude 2)' },
Expand Down Expand Up @@ -541,6 +550,10 @@ export const defaultConfig = {
'openRouter_anthropic_claude_sonnet4',
'openRouter_google_gemini_2_5_pro',
'openRouter_openai_o3',
'chatgptApiO4Mini',
'chatgptApiGpt5',
'chatgptApiGpt5Mini',
'chatgptApiGpt5Nano',
],
customApiModes: [
{
Expand Down
124 changes: 97 additions & 27 deletions src/services/apis/openai-api.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import { fetchSSE } from '../../utils/fetch-sse.mjs'
import { getConversationPairs } from '../../utils/get-conversation-pairs.mjs'
import { isEmpty } from 'lodash-es'
import { getCompletionPromptBase, pushRecord, setAbortController } from './shared.mjs'
import { getModelValue } from '../../utils/model-name-convert.mjs'
import { getModelValue, isUsingReasoningModel } from '../../utils/model-name-convert.mjs'

/**
* @param {Browser.Runtime.Port} port
Expand Down Expand Up @@ -65,10 +65,16 @@ export async function generateAnswersWithGptCompletionApi(port, question, sessio
return
}

answer += data.choices[0].text
const choice = data.choices?.[0]
if (!choice) {
console.debug('No choice in response data')
return
}

answer += choice.text
port.postMessage({ answer: answer, done: false, session: null })

if (data.choices[0]?.finish_reason) {
if (choice.finish_reason) {
finish()
return
}
Expand Down Expand Up @@ -116,37 +122,64 @@ export async function generateAnswersWithChatgptApiCompat(
) {
const { controller, messageListener, disconnectListener } = setAbortController(port)
const model = getModelValue(session)
const isReasoningModel = isUsingReasoningModel(session)

const config = await getUserConfig()
const prompt = getConversationPairs(
session.conversationRecords.slice(-config.maxConversationContextLength),
false,
)
prompt.push({ role: 'user', content: question })

// Filter messages based on model type
const filteredPrompt = isReasoningModel
? prompt.filter((msg) => msg.role === 'user' || msg.role === 'assistant')
: prompt

Comment thread
PeterDaveHello marked this conversation as resolved.
filteredPrompt.push({ role: 'user', content: question })

let answer = ''
let finished = false
const finish = () => {
if (finished) return
finished = true
Comment on lines 176 to 179
Copy link

Copilot AI Aug 30, 2025

Choose a reason for hiding this comment

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

The finish function checks and sets the 'finished' flag without synchronization, which could lead to race conditions in concurrent scenarios. Consider using atomic operations or proper synchronization to ensure thread safety.

Suggested change
let finished = false
const finish = () => {
if (finished) return
finished = true
// Use an atomic flag for thread safety
const finishedBuffer = new SharedArrayBuffer(4);
const finishedFlag = new Int32Array(finishedBuffer);
const finish = () => {
// Atomically check and set the finished flag
if (Atomics.compareExchange(finishedFlag, 0, 0, 1) !== 0) return;

Copilot uses AI. Check for mistakes.
pushRecord(session, question, answer)
console.debug('conversation history', { content: session.conversationRecords })
port.postMessage({ answer: null, done: true, session: session })
port.postMessage({ answer: null, done: true, session })
}
Comment thread
PeterDaveHello marked this conversation as resolved.

// Build request body with reasoning model-specific parameters
const requestBody = {
messages: filteredPrompt,
model,
...extraBody,
}

if (isReasoningModel) {
// Reasoning models use max_completion_tokens instead of max_tokens
requestBody.max_completion_tokens = config.maxResponseTokenLength
// Reasoning models don't support streaming during beta
requestBody.stream = false
// Reasoning models have fixed parameters during beta
requestBody.temperature = 1
requestBody.top_p = 1
requestBody.n = 1
requestBody.presence_penalty = 0
requestBody.frequency_penalty = 0
} else {
// Non-reasoning models use the existing behavior
requestBody.stream = true
requestBody.max_tokens = config.maxResponseTokenLength
requestBody.temperature = config.temperature
}

Comment on lines +150 to +216
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Aug 24, 2025

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Enforce reasoning-model limitations by stripping tools/functions from requestBody

extraBody may inject tools or function-calls. Reasoning endpoints typically reject or ignore them; explicitly removing avoids confusing API errors.

Apply this diff:

   if (isReasoningModel) {
     // Reasoning models use max_completion_tokens instead of max_tokens
     requestBody.max_completion_tokens = config.maxResponseTokenLength
     // Reasoning models don't support streaming during beta
     requestBody.stream = false
     // Reasoning models have fixed parameters during beta
     requestBody.temperature = 1
     requestBody.top_p = 1
     requestBody.n = 1
     requestBody.presence_penalty = 0
     requestBody.frequency_penalty = 0
+    // Disallow tools/functions/function calling in reasoning mode
+    delete requestBody.tools
+    delete requestBody.tool_choice
+    delete requestBody.functions
+    delete requestBody.function_call
   } else {
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// Build request body with reasoning model-specific parameters
const requestBody = {
messages: filteredPrompt,
model,
...extraBody,
}
if (isReasoningModel) {
// Reasoning models use max_completion_tokens instead of max_tokens
requestBody.max_completion_tokens = config.maxResponseTokenLength
// Reasoning models don't support streaming during beta
requestBody.stream = false
// Reasoning models have fixed parameters during beta
requestBody.temperature = 1
requestBody.top_p = 1
requestBody.n = 1
requestBody.presence_penalty = 0
requestBody.frequency_penalty = 0
} else {
// Non-reasoning models use the existing behavior
requestBody.stream = true
requestBody.max_tokens = config.maxResponseTokenLength
requestBody.temperature = config.temperature
}
// Build request body with reasoning model-specific parameters
const requestBody = {
messages: filteredPrompt,
model,
...extraBody,
}
if (isReasoningModel) {
// Reasoning models use max_completion_tokens instead of max_tokens
requestBody.max_completion_tokens = config.maxResponseTokenLength
// Reasoning models don't support streaming during beta
requestBody.stream = false
// Reasoning models have fixed parameters during beta
requestBody.temperature = 1
requestBody.top_p = 1
requestBody.n = 1
requestBody.presence_penalty = 0
requestBody.frequency_penalty = 0
// Disallow tools/functions/function calling in reasoning mode
delete requestBody.tools
delete requestBody.tool_choice
delete requestBody.functions
delete requestBody.function_call
} else {
// Non-reasoning models use the existing behavior
requestBody.stream = true
requestBody.max_tokens = config.maxResponseTokenLength
requestBody.temperature = config.temperature
}
🤖 Prompt for AI Agents 
In src/services/apis/openai-api.mjs around lines 150 to 174, extraBody can
inject tools/functions/function_call which reasoning endpoints reject; when
isReasoningModel is true, remove any fields like functions, tools, function_call
(and any nested tool-related entries) from requestBody/extraBody before merging
so the built requestBody does not include them; implement this by sanitizing
extraBody (delete or omit keys "functions", "tools", "function_call" and any
tool-specific keys) prior to spreading into requestBody in the reasoning-model
branch, then set the reasoning-specific fields as shown.

await fetchSSE(`${baseUrl}/chat/completions`, {
method: 'POST',
signal: controller.signal,
headers: {
'Content-Type': 'application/json',
Authorization: `Bearer ${apiKey}`,
},
body: JSON.stringify({
messages: prompt,
model,
stream: true,
max_tokens: config.maxResponseTokenLength,
temperature: config.temperature,
...extraBody,
}),
body: JSON.stringify(requestBody),
Comment on lines 139 to +231
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot Aug 29, 2025

Choose a reason for hiding this comment

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

[P1] Handle multi‑chunk JSON for non‑streaming reasoning responses

Setting requestBody.stream = false for reasoning models still routes the call through fetchSSE, which only synthesizes SSE events when the entire JSON payload fits in the first ReadableStream chunk. Large reasoning responses are likely to span multiple chunks; in that case fetchSSE never parses the plain JSON body and onMessage is never invoked, so answer stays empty and finish() is not called. Users will see an empty reply followed only by { done: true }. Consider either enabling streaming or reading the full JSON body when stream is false before calling onMessage.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Member

@PeterDaveHello PeterDaveHello Aug 30, 2025

Choose a reason for hiding this comment

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

@copilot, address this feedback.

Copy link
Copy Markdown
Contributor

@qodo-code-review qodo-code-review Bot Aug 30, 2025

Choose a reason for hiding this comment

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

Code Implementation 🛠️

Implementation: Modify the request flow to bypass fetchSSE when requestBody.stream is false (reasoning models). Perform a standard fetch, read and parse the complete JSON response, and then invoke the existing onMessage and finish logic once. This ensures large non-streaming reasoning responses that span multiple chunks are handled correctly.

Suggested change
'Content-Type': 'application/json',
Authorization: `Bearer ${apiKey}`,
},
body: JSON.stringify({
messages: prompt,
model,
stream: true,
max_tokens: config.maxResponseTokenLength,
temperature: config.temperature,
...extraBody,
}),
body: JSON.stringify(requestBody),
headers: {
'Content-Type': 'application/json',
Authorization: `Bearer ${apiKey}`,
},
body: JSON.stringify(requestBody),
// For non-streaming reasoning models, fetch the full JSON response instead of using SSE
...(isReasoningModel && requestBody.stream === false
? {
async onStart() {
// No-op: handled entirely in onEnd via standard fetch below
},
async onMessage() {
// No-op for non-streaming path
},
async onEnd() {},
}
: {
onMessage(message) {
console.debug('sse message', message)
if (finished) return
if (message.trim() === '[DONE]') {
finish()
return
}
let data
try {
data = JSON.parse(message)
} catch (error) {
console.debug('json error', error)
return
}
// Streaming (non-reasoning) path
const choice = data.choices?.[0]
if (!choice) {
console.debug('No choice in response data')
return
}
const delta = choice.delta?.content
const content = choice.message?.content
const text = choice.text
if (delta !== undefined) {
answer += delta
} else if (content) {
answer = content
} else if (text) {
answer += text
}
port.postMessage({ answer, done: false, session: null })
if (choice.finish_reason) {
finish()
return
}
},
}),
})
// If non-streaming reasoning model, perform a standard fetch to read full JSON
if (isReasoningModel && requestBody.stream === false) {
try {
const resp = await fetch(`${baseUrl}/chat/completions`, {
method: 'POST',
signal: controller.signal,
headers: {
'Content-Type': 'application/json',
Authorization: `Bearer ${apiKey}`,
},
body: JSON.stringify(requestBody),
})
if (!resp.ok) {
const error = await resp.json().catch(() => ({}))
throw new Error(!isEmpty(error) ? JSON.stringify(error) : `${resp.status} ${resp.statusText}`)
}
const data = await resp.json()
const choice = data.choices?.[0]
if (choice) {
let content = choice.message?.content ?? choice.text
if (Array.isArray(content)) {
const parts = content
.map((p) => {
if (typeof p === 'string') return p
if (p && typeof p === 'object') {
if (typeof p.output_text === 'string') return p.output_text
if (typeof p.text === 'string') return p.text
}
return ''
})
.filter(Boolean)
content = parts.join('')
}
if (content !== undefined && content !== null) {
answer = String(content)
port.postMessage({ answer, done: false, session: null })
}
}
finish()
} catch (resp) {
port.onMessage.removeListener(messageListener)
port.onDisconnect.removeListener(disconnectListener)
if (resp instanceof Error) throw resp
const error = await resp.json().catch(() => ({}))
throw new Error(!isEmpty(error) ? JSON.stringify(error) : `${resp.status} ${resp.statusText}`)
} finally {
port.postMessage({ done: true })
port.onMessage.removeListener(messageListener)
port.onDisconnect.removeListener(disconnectListener)
}
return
}

See review comment here

Copy link
Copy Markdown
Contributor Author

Copilot AI Aug 30, 2025

Choose a reason for hiding this comment

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

Added API key validation to prevent runtime errors from invalid or empty API keys. The Authorization header now uses the trimmed API key and validates input before making the request.

Commit: f878898

onMessage(message) {
console.debug('sse message', message)
if (finished) return
Expand All @@ -162,21 +195,58 @@ export async function generateAnswersWithChatgptApiCompat(
return
}

const delta = data.choices[0]?.delta?.content
const content = data.choices[0]?.message?.content
const text = data.choices[0]?.text
if (delta !== undefined) {
answer += delta
} else if (content) {
answer = content
} else if (text) {
answer += text
}
port.postMessage({ answer: answer, done: false, session: null })
if (isReasoningModel) {
// For reasoning models (non-streaming), get the complete response
const choice = data.choices?.[0]
if (!choice) {
console.debug('No choice in response data for reasoning model')
return
}
let content = choice.message?.content ?? choice.text
if (Array.isArray(content)) {
// Prefer output_text segments; fallback to any string content
const parts = content
.map((p) => {
if (typeof p === 'string') return p
if (p && typeof p === 'object') {
if (typeof p.output_text === 'string') return p.output_text
if (typeof p.text === 'string') return p.text
}
return ''
})
.filter(Boolean)
content = parts.join('')
}
Comment on lines +206 to +261
Copy link

Copilot AI Aug 29, 2025

Choose a reason for hiding this comment

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

[nitpick] The array content processing logic is complex and nested. Consider extracting this into a separate helper function extractTextFromContentArray() to improve readability and reusability.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Member

@PeterDaveHello PeterDaveHello Aug 30, 2025

Choose a reason for hiding this comment

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

@copilot, address this feedback.

Copy link
Copy Markdown
Contributor Author

Copilot AI Aug 30, 2025

Choose a reason for hiding this comment

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

Improved the content processing logic by extracting a dedicated helper function extractContentFromArray with enhanced error handling. The code now has better separation of concerns, improved readability, and more robust processing of structured response arrays from reasoning models.

Commit: f878898

Comment on lines +211 to +261
Copy link

Copilot AI Aug 29, 2025

Choose a reason for hiding this comment

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

[nitpick] The nested content processing logic is complex and could be extracted into a separate function for better readability and testability. Consider creating a helper function like extractContentFromArray(contentArray) to handle the array parsing logic.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Member

@PeterDaveHello PeterDaveHello Aug 30, 2025

Choose a reason for hiding this comment

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

@copilot, address this feedback.

Copy link
Copy Markdown
Contributor Author

Copilot AI Aug 30, 2025

Choose a reason for hiding this comment

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

Enhanced the finish condition logic to only complete when there's a proper finish_reason, preventing premature completion. Also improved content validation to ensure only valid, non-empty string content is processed and sent to the UI.

Commit: f878898

if (content !== undefined && content !== null) {
answer = String(content)
port.postMessage({ answer, done: false, session: null })
}
if (choice.finish_reason || content !== undefined) {
finish()
Comment thread
PeterDaveHello marked this conversation as resolved.
}
} else {
// For non-reasoning models (streaming), handle delta content
const choice = data.choices?.[0]
if (!choice) {
console.debug('No choice in response data')
return
}
const delta = choice.delta?.content
const content = choice.message?.content
const text = choice.text
if (delta !== undefined) {
answer += delta
} else if (content) {
answer = content
} else if (text) {
answer += text
}
port.postMessage({ answer, done: false, session: null })

Comment thread
PeterDaveHello marked this conversation as resolved.
if (data.choices[0]?.finish_reason) {
finish()
return
if (choice.finish_reason) {
finish()
return
}
}
},
async onStart() {},
Expand Down
17 changes: 17 additions & 0 deletions src/utils/model-name-convert.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -164,3 +164,20 @@ export function isInApiModeGroup(apiModeGroup, configOrSession) {
const [, { value: groupValue }] = foundGroup
return groupValue === apiModeGroup
}

export function isUsingReasoningModel(configOrSession) {
const modelValue = getModelValue(configOrSession)
if (!modelValue) return false

// Explicitly match o1, o3, or o4 with optional dash and suffix (e.g., o1, o1-preview, o3-mini, o4-mini)
if (/^(o1|o3|o4)(?:-|$)/.test(modelValue)) {
Copy link

Copilot AI Aug 24, 2025

Choose a reason for hiding this comment

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

The regex pattern (?:-|$) will match models like 'o1' or 'o4' but will also incorrectly match invalid models like 'o15' or 'o44'. The pattern should be more restrictive to only match the intended model patterns.

Suggested change
// Explicitly match o1, o3, or o4 with optional dash and suffix (e.g., o1, o1-preview, o3-mini, o4-mini)
if (/^(o1|o3|o4)(?:-|$)/.test(modelValue)) {
// Explicitly match o1, o3, or o4 as standalone or with a dash and a valid suffix (e.g., o1, o1-preview, o3-mini, o4-mini), but not o15, o44, etc.
if (/^(o1|o3|o4)(?:$|-[a-zA-Z][\w-]*)$/.test(modelValue)) {

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Member

@PeterDaveHello PeterDaveHello Aug 29, 2025

Choose a reason for hiding this comment

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

@codex address this feedback

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot Aug 29, 2025

Choose a reason for hiding this comment

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

@codex address this feedback (path=src/utils/model-name-convert.mjs line=173 side=RIGHT)

For now, I can only help with PRs you've created.

Copy link
Copy Markdown
Member

@PeterDaveHello PeterDaveHello Aug 29, 2025

Choose a reason for hiding this comment

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

@copilot, address this feedback.

return true
}

// Match gpt-5* pattern but exclude gpt-5-chat-* variants
if (modelValue.startsWith('gpt-5') && !modelValue.startsWith('gpt-5-chat')) {
return true
}

return false
}
Comment on lines +168 to +195
Copy link

Copilot AI Aug 23, 2025

Choose a reason for hiding this comment

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

The function name 'isUsingReasoningModel' is misleading since it only checks for specific future models but excludes existing reasoning models like 'o1-preview', 'o1-mini', and 'o3-mini'. Consider renaming to 'isUsingNewReasoningModel' or including all reasoning models for consistency.

Copilot uses AI. Check for mistakes.
Comment on lines +168 to +195
Copy link

Copilot AI Aug 29, 2025

Choose a reason for hiding this comment

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

The regex pattern ^(o1|o3|o4)(?:-|$) will incorrectly match model names like 'o10', 'o11', 'o30', 'o40', etc. The pattern should use word boundaries or be more specific to match only the exact model numbers followed by a dash or end of string.

Copilot uses AI. Check for mistakes.