fix(ai): add optional reasoning support for GPT-5 compatible APIs#63
fix(ai): add optional reasoning support for GPT-5 compatible APIs#63AmintaCCCP merged 2 commits intomainfrom
Conversation
📝 WalkthroughWalkthroughAdds persistent "reasoning effort" for AI configs and threads it through API routes, DB schema, frontend settings, service logic, and the proxy so OpenAI-compatible reasoning blocks can be conditionally injected and timeouts adjusted. Changes
Sequence DiagramssequenceDiagram
participant User as User
participant UI as SettingsPanel
participant API as API Routes
participant DB as Database
participant Service as aiService
participant Proxy as Proxy
participant OpenAI as OpenAI API
User->>UI: Select reasoning effort and save
UI->>API: PUT /api/configs/ai/:id { reasoningEffort }
API->>DB: UPDATE ai_configs SET reasoning_effort = ?
DB-->>API: OK
API-->>UI: Success
User->>Service: Request AI response
Service->>DB: SELECT ai_config WHERE id=...
DB-->>Service: { apiType, reasoning_effort }
Service->>Service: build request body (include reasoning if applicable)
Service->>Proxy: POST with effective body, timeout based on reasoning/apiType
Proxy->>OpenAI: Forward request (may include reasoning)
OpenAI-->>Proxy: Response
Proxy-->>Service: Response
Service-->>User: Final result
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/types/index.ts`:
- Around line 70-71: Update the AIReasoningEffort union type to include the
missing OpenAI-supported values so it matches the API spec: modify the exported
type AIReasoningEffort (in src/types/index.ts) to add 'none' and 'xhigh'
alongside the existing 'minimal' | 'low' | 'medium' | 'high' entries.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9efb49c5-cb22-4458-8de5-1354b2162ec2
📒 Files selected for processing (7)
server/src/db/schema.tsserver/src/routes/configs.tsserver/src/routes/proxy.tsserver/src/routes/sync.tssrc/components/SettingsPanel.tsxsrc/services/aiService.tssrc/types/index.ts
src/types/index.ts
Outdated
| export type AIApiType = 'openai' | 'openai-responses' | 'claude' | 'gemini'; | ||
| export type AIReasoningEffort = 'minimal' | 'low' | 'medium' | 'high'; |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
OpenAI API reasoning effort parameter values
💡 Result:
The OpenAI API reasoning effort parameter, named reasoning_effort in the Chat Completions API (/v1/chat/completions) and reasoning.effort in the Responses API (/v1/responses), controls the amount of computational effort reasoning models (e.g., o1 series, gpt-5 series) dedicate to internal reasoning before generating a response. Lower values prioritize speed and lower token usage; higher values enable more thorough reasoning at the cost of latency and tokens. Supported values (model-dependent): - none: No reasoning (fastest, lowest tokens; default for gpt-5.1+ like gpt-5.4). - minimal: Minimal reasoning (some models). - low: Light reasoning. - medium: Balanced (default for pre-gpt-5.1 models). - high: Thorough reasoning. - xhigh: Maximum reasoning (later models). Early o1 models supported low, medium, high. Newer models expand options. Use max_completion_tokens (not max_tokens) for reasoning models. Unsupported with some params like temperature when reasoning > none. Example (Chat Completions): openai.chat.completions.create( model="o1", messages=[...], reasoning_effort="high" ) Example (Responses): openai.responses.create( model="gpt-5.4", reasoning={"effort": "medium"}, input=[...] )
Citations:
- 1: http://developers.openai.com/api/docs/api-reference/chat/create
- 2: https://community.openai.com/t/o1s-reasoning-effort-parameter/1062308
- 3: https://developers.openai.com/docs/guides/reasoning
- 4: https://platform.openai.com/docs/guides/latest-model?reasoning-effort-mode=chat&verbosity-mode=chat
- 5: https://developers.openai.com/api/reference/resources/responses/
- 6: https://platform.openai.com/docs/guides/reasoning?api-mode=chat
Add missing reasoning effort values to match OpenAI API specification.
The AIReasoningEffort type is incomplete. The OpenAI API supports 'none' (default for newer models) and 'xhigh' (maximum reasoning for later models) in addition to the currently defined values. Update to:
export type AIReasoningEffort = 'none' | 'minimal' | 'low' | 'medium' | 'high' | 'xhigh';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/types/index.ts` around lines 70 - 71, Update the AIReasoningEffort union
type to include the missing OpenAI-supported values so it matches the API spec:
modify the exported type AIReasoningEffort (in src/types/index.ts) to add 'none'
and 'xhigh' alongside the existing 'minimal' | 'low' | 'medium' | 'high'
entries.
|
Follow-up fix pushed. The first version used
This update fixes that by:
Frontend build re-checked with |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
src/components/SettingsPanel.tsx (1)
820-842: Consider gating Reasoning Effort by API type to avoid confusing no-op configs.The control is editable for Claude/Gemini even though it only applies to OpenAI-compatible endpoints. Disabling or hiding it for non-OpenAI types would prevent misleading settings.
Suggested refactor
+ const isOpenAICompatible = aiForm.apiType === 'openai' || aiForm.apiType === 'openai-responses'; ... - <div> + <div> <label className="block text-sm font-medium text-gray-700 dark:text-gray-300 mb-1"> {t('推理强度', 'Reasoning Effort')} </label> <select value={aiForm.reasoningEffort} onChange={(e) => setAIForm(prev => ({ ...prev, reasoningEffort: e.target.value as '' | AIReasoningEffort }))} + disabled={!isOpenAICompatible} className="w-full px-3 py-2 border border-gray-300 dark:border-gray-600 rounded-lg bg-white dark:bg-gray-800 text-gray-900 dark:text-white" >Also clear stale value when switching away from OpenAI-compatible
apiType.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/SettingsPanel.tsx` around lines 820 - 842, The Reasoning Effort select (using aiForm.reasoningEffort and setAIForm) should be hidden or disabled unless the current api type is OpenAI-compatible; update the component rendering around the select to check the API type (e.g., apiType / isOpenAICompatible flag) and only render or enable the control for OpenAI-compatible endpoints, and when switching apiType away from OpenAI-compatible clear the stale aiForm.reasoningEffort value via setAIForm(prev => ({ ...prev, reasoningEffort: '' })) so non-applicable settings are removed.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@server/src/routes/proxy.ts`:
- Line 134: The timeout is currently set to 10 minutes whenever reasoningEffort
is truthy regardless of provider; change the conditional so the 10-minute path
is only used when apiType === 'openai-responses' AND reasoningEffort is truthy.
Update the expression that sets the timeout (the const timeout assignment using
apiType and reasoningEffort) to use a strict conjunction (apiType ===
'openai-responses' && !!reasoningEffort) ? 600000 : 60000 so non-OpenAI
providers never get the extended 10-minute hold.
- Around line 96-98: The current mapping only remaps 'minimal' and forwards any
other aiConfig.reasoning_effort which can cause upstream 4xxs; normalize
aiConfig.reasoning_effort (use the aiConfig.reasoning_effort symbol) against an
explicit allowed set (e.g., 'low','medium','high'), map 'minimal' to 'low', and
if the resulting value is not in that allowed set set reasoningEffort to
null/undefined so unknown values are dropped before being forwarded (update the
reasoningEffort assignment in server/src/routes/proxy.ts to perform this check).
In `@src/components/SettingsPanel.tsx`:
- Line 398: When merging restored backup settings, don't copy
cfg.reasoningEffort directly; instead normalize/validate it before assigning to
reasoningEffort so legacy or invalid values (e.g., 'minimal') are converted to
the current allowed enum. Replace the direct assignment of cfg.reasoningEffort
with a call to the same normalization routine used elsewhere (e.g.,
normalizeReasoningEffort(cfg.reasoningEffort)) or inline the same
validation/mapping logic so the restored value is coerced to a valid runtime
option.
---
Nitpick comments:
In `@src/components/SettingsPanel.tsx`:
- Around line 820-842: The Reasoning Effort select (using aiForm.reasoningEffort
and setAIForm) should be hidden or disabled unless the current api type is
OpenAI-compatible; update the component rendering around the select to check the
API type (e.g., apiType / isOpenAICompatible flag) and only render or enable the
control for OpenAI-compatible endpoints, and when switching apiType away from
OpenAI-compatible clear the stale aiForm.reasoningEffort value via
setAIForm(prev => ({ ...prev, reasoningEffort: '' })) so non-applicable settings
are removed.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7c104611-a7d7-47ae-94a3-065cfc7baeb0
📒 Files selected for processing (4)
server/src/routes/proxy.tssrc/components/SettingsPanel.tsxsrc/services/aiService.tssrc/types/index.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- src/types/index.ts
- src/services/aiService.ts
| const reasoningEffort = aiConfig.reasoning_effort === 'minimal' | ||
| ? 'low' | ||
| : aiConfig.reasoning_effort as string | null | undefined; |
There was a problem hiding this comment.
Normalize reasoningEffort against an allowed set before use.
At Line 96, only 'minimal' is remapped. Other unexpected persisted values are still forwarded and can cause upstream 4xx failures. Normalize to allowed values and drop unknowns.
Suggested fix
- const reasoningEffort = aiConfig.reasoning_effort === 'minimal'
- ? 'low'
- : aiConfig.reasoning_effort as string | null | undefined;
+ const rawReasoningEffort = typeof aiConfig.reasoning_effort === 'string'
+ ? aiConfig.reasoning_effort.trim().toLowerCase()
+ : undefined;
+ const allowedReasoningEfforts = new Set(['none', 'low', 'medium', 'high', 'xhigh']);
+ const reasoningEffort = rawReasoningEffort === 'minimal'
+ ? 'low'
+ : (rawReasoningEffort && allowedReasoningEfforts.has(rawReasoningEffort) ? rawReasoningEffort : undefined);📝 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.
| const reasoningEffort = aiConfig.reasoning_effort === 'minimal' | |
| ? 'low' | |
| : aiConfig.reasoning_effort as string | null | undefined; | |
| const rawReasoningEffort = typeof aiConfig.reasoning_effort === 'string' | |
| ? aiConfig.reasoning_effort.trim().toLowerCase() | |
| : undefined; | |
| const allowedReasoningEfforts = new Set(['none', 'low', 'medium', 'high', 'xhigh']); | |
| const reasoningEffort = rawReasoningEffort === 'minimal' | |
| ? 'low' | |
| : (rawReasoningEffort && allowedReasoningEfforts.has(rawReasoningEffort) ? rawReasoningEffort : undefined); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@server/src/routes/proxy.ts` around lines 96 - 98, The current mapping only
remaps 'minimal' and forwards any other aiConfig.reasoning_effort which can
cause upstream 4xxs; normalize aiConfig.reasoning_effort (use the
aiConfig.reasoning_effort symbol) against an explicit allowed set (e.g.,
'low','medium','high'), map 'minimal' to 'low', and if the resulting value is
not in that allowed set set reasoningEffort to null/undefined so unknown values
are dropped before being forwarded (update the reasoningEffort assignment in
server/src/routes/proxy.ts to perform this check).
| ? { ...requestBody, reasoning: { effort: reasoningEffort } } | ||
| : requestBody; | ||
|
|
||
| const timeout = apiType === 'openai-responses' || !!reasoningEffort ? 600000 : 60000; |
There was a problem hiding this comment.
Scope the 10-minute timeout to OpenAI-compatible reasoning paths only.
Line 134 currently extends timeout whenever reasoningEffort is truthy, even for non-OpenAI providers where no reasoning payload is sent. This can unnecessarily hold proxy resources.
Suggested fix
- const timeout = apiType === 'openai-responses' || !!reasoningEffort ? 600000 : 60000;
+ const isOpenAICompatible = apiType === 'openai' || apiType === 'openai-responses';
+ const usesReasoning = isOpenAICompatible && !!reasoningEffort;
+ const timeout = apiType === 'openai-responses' || usesReasoning ? 600000 : 60000;📝 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.
| const timeout = apiType === 'openai-responses' || !!reasoningEffort ? 600000 : 60000; | |
| const isOpenAICompatible = apiType === 'openai' || apiType === 'openai-responses'; | |
| const usesReasoning = isOpenAICompatible && !!reasoningEffort; | |
| const timeout = apiType === 'openai-responses' || usesReasoning ? 600000 : 60000; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@server/src/routes/proxy.ts` at line 134, The timeout is currently set to 10
minutes whenever reasoningEffort is truthy regardless of provider; change the
conditional so the 10-minute path is only used when apiType ===
'openai-responses' AND reasoningEffort is truthy. Update the expression that
sets the timeout (the const timeout assignment using apiType and
reasoningEffort) to use a strict conjunction (apiType === 'openai-responses' &&
!!reasoningEffort) ? 600000 : 60000 so non-OpenAI providers never get the
extended 10-minute hold.
| customPrompt: cfg.customPrompt, | ||
| useCustomPrompt: cfg.useCustomPrompt, | ||
| concurrency: cfg.concurrency, | ||
| reasoningEffort: cfg.reasoningEffort, |
There was a problem hiding this comment.
Normalize restored reasoningEffort during backup merge.
At Line 398, restored values are copied as-is. Legacy backups containing 'minimal' (or invalid values) will remain in state and UI. Normalize here as well to keep behavior consistent without requiring manual edit/re-save.
Suggested fix
- reasoningEffort: cfg.reasoningEffort,
+ reasoningEffort:
+ cfg.reasoningEffort === 'minimal'
+ ? 'low'
+ : (['none', 'low', 'medium', 'high', 'xhigh'].includes(cfg.reasoningEffort || '')
+ ? cfg.reasoningEffort
+ : undefined),📝 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.
| reasoningEffort: cfg.reasoningEffort, | |
| reasoningEffort: | |
| cfg.reasoningEffort === 'minimal' | |
| ? 'low' | |
| : (['none', 'low', 'medium', 'high', 'xhigh'].includes(cfg.reasoningEffort || '') | |
| ? cfg.reasoningEffort | |
| : undefined), |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/SettingsPanel.tsx` at line 398, When merging restored backup
settings, don't copy cfg.reasoningEffort directly; instead normalize/validate it
before assigning to reasoningEffort so legacy or invalid values (e.g.,
'minimal') are converted to the current allowed enum. Replace the direct
assignment of cfg.reasoningEffort with a call to the same normalization routine
used elsewhere (e.g., normalizeReasoningEffort(cfg.reasoningEffort)) or inline
the same validation/mapping logic so the restored value is coerced to a valid
runtime option.
1 participant
Summary
reasoningEffortsupport for OpenAI-compatible AI configs while keeping legacy behavior unchanged by defaultreasoning: { effort }for OpenAI Chat Completions and Responses only when configuredWhy
Issue #62 reports that GPT-5 series models may require a
reasoningparameter, which is not supported in v0.2.6.PR #44 added Responses API support, but did not actually add
reasoningconfiguration or request forwarding.This PR fixes that gap while remaining backward compatible:
reasoningValidation
npm run build✅better-sqlite3install fails under node-gyp/python tooling, but the changed server paths were reviewed for request/config/schema consistencyCloses #62
Summary by CodeRabbit