🐛 fix: respect user's enableResponseApi setting in server-side SubAgent task execution #11320
+16
−0
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@Ricky-Hao is attempting to deploy a commit to the LobeHub OSS Team on Vercel. A member of the Team first needs to authorize it. |
Contributor
Reviewer's guide (collapsed on small PRs)Reviewer's GuideEnsures server-side SubAgent task execution respects the user's provider configuration for Responses API by deriving an explicit apiMode from the stored provider config and passing it through to the chat payload. Sequence diagram for server-side SubAgent task execution respecting enableResponseApisequenceDiagram
actor SubAgent
participant RuntimeExecutors
participant AiProviderModel
participant ServerDB
participant ModelRuntime
participant ProviderAPI
SubAgent->>RuntimeExecutors: executeSubAgentTask(provider, llmPayload)
RuntimeExecutors->>ModelRuntime: initModelRuntimeFromDB(ServerDB, userId, provider)
ModelRuntime-->>RuntimeExecutors: modelRuntime
RuntimeExecutors->>AiProviderModel: constructor(ServerDB, userId)
RuntimeExecutors->>AiProviderModel: findById(provider)
AiProviderModel->>ServerDB: query provider config
ServerDB-->>AiProviderModel: providerConfig(config.enableResponseApi)
AiProviderModel-->>RuntimeExecutors: providerConfig
RuntimeExecutors->>RuntimeExecutors: determine apiMode
Note over RuntimeExecutors: apiMode = responses if enableResponseApi === true
Note over RuntimeExecutors: apiMode = chatCompletion otherwise
RuntimeExecutors->>RuntimeExecutors: build chatPayload(apiMode, messages, model, tools)
RuntimeExecutors->>ProviderAPI: send chatPayload
ProviderAPI-->>RuntimeExecutors: response
RuntimeExecutors-->>SubAgent: task result
Flow diagram for apiMode selection based on enableResponseApiflowchart TD
A[Start SubAgent task execution] --> B[Load providerConfig from database]
B --> C{enableResponseApi === true?}
C -- Yes --> D[Set apiMode to responses]
C -- No --> E[Set apiMode to chatCompletion]
D --> F[Construct chatPayload with apiMode]
E --> F[Construct chatPayload with apiMode]
F --> G[Send chatPayload to provider]
G --> H[End]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
dosubot
bot
added
the
size:S
Contributor
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- The comment about defaulting to false for non-OpenAI providers no longer matches the implementation (which defaults to chatCompletion for all providers unless explicitly enabled), so consider updating or removing it to avoid confusion.
- Since the provider config is now fetched here in addition to initModelRuntimeFromDB, consider centralizing the apiMode derivation in a shared helper (or exposing it from initModelRuntimeFromDB) to avoid duplicated provider-configuration logic.
- It might be worth handling or logging the case where providerConfig is unexpectedly missing so that misconfigured providers are easier to diagnose rather than silently falling back to chatCompletion.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The comment about defaulting to false for non-OpenAI providers no longer matches the implementation (which defaults to chatCompletion for all providers unless explicitly enabled), so consider updating or removing it to avoid confusion.
- Since the provider config is now fetched here in addition to initModelRuntimeFromDB, consider centralizing the apiMode derivation in a shared helper (or exposing it from initModelRuntimeFromDB) to avoid duplicated provider-configuration logic.
- It might be worth handling or logging the case where providerConfig is unexpectedly missing so that misconfigured providers are easier to diagnose rather than silently falling back to chatCompletion.
## Individual Comments
### Comment 1
<location> `src/server/modules/AgentRuntime/RuntimeExecutors.ts:132-134` </location>
<code_context>
+ // When user explicitly disables Responses API, set apiMode to 'chatCompletion'
+ const aiProviderModel = new AiProviderModel(ctx.serverDB, ctx.userId!);
+ const providerConfig = await aiProviderModel.findById(provider);
+ const enableResponseApi = providerConfig?.config?.enableResponseApi;
+ // Default to false for non-OpenAI providers, true only for OpenAI with explicit enable
+ const apiMode: 'responses' | 'chatCompletion' =
+ enableResponseApi === true ? 'responses' : 'chatCompletion';
+
</code_context>
<issue_to_address>
**issue:** Align the comment about non-OpenAI defaults with the actual apiMode computation.
The logic currently enables `responses` only when `enableResponseApi === true`, regardless of provider. There’s no OpenAI vs non‑OpenAI distinction in the code, so the comment describing different defaults per provider is misleading. Please either adjust the comment to match this behavior or add the missing provider check if that behavior is intended.
</issue_to_address>
### Comment 2
<location> `src/server/modules/AgentRuntime/RuntimeExecutors.ts:130-131` </location>
<code_context>
+ // Read user's provider config to determine apiMode
+ // When user explicitly disables Responses API, set apiMode to 'chatCompletion'
+ const aiProviderModel = new AiProviderModel(ctx.serverDB, ctx.userId!);
+ const providerConfig = await aiProviderModel.findById(provider);
+ const enableResponseApi = providerConfig?.config?.enableResponseApi;
+ // Default to false for non-OpenAI providers, true only for OpenAI with explicit enable
</code_context>
<issue_to_address>
**suggestion (performance):** Consider avoiding an extra DB round-trip for providerConfig on every execution.
This adds a second DB query via `AiProviderModel.findById` immediately after `initModelRuntimeFromDB`. If `initModelRuntimeFromDB` already loads (or can be extended to load) provider config, we could derive `apiMode` from that data instead and avoid the extra query on the hot path, reducing latency and DB load for frequent requests.
Suggested implementation:
```typescript
import { type MessageModel } from '@/database/models/message';
import { type LobeChatDatabase } from '@/database/type';
import { initModelRuntimeFromDB } from '@/server/modules/ModelRuntime';
// 初始化 ModelRuntime (从数据库读取用户的 keyVaults)
const modelRuntime = await initModelRuntimeFromDB(ctx.serverDB, ctx.userId!, provider);
// Read user's provider config (already loaded by initModelRuntimeFromDB) to determine apiMode
// When user explicitly disables Responses API, set apiMode to 'chatCompletion'
const enableResponseApi = modelRuntime.providerConfig?.config?.enableResponseApi;
// Default to false for non-OpenAI providers, true only for OpenAI with explicit enable
const apiMode: 'responses' | 'chatCompletion' =
enableResponseApi === true ? 'responses' : 'chatCompletion';
// 构造 ChatStreamPayload
const chatPayload = {
```
To fully implement this optimization, you also need to:
1. Update `initModelRuntimeFromDB` (in `src/server/modules/ModelRuntime.ts` or its actual location) so that the returned `modelRuntime` instance includes the provider configuration:
- Add a `providerConfig` (or similarly named) property to the `ModelRuntime` type/interface.
- Inside `initModelRuntimeFromDB`, load the provider config (using `AiProviderModel` or equivalent) and assign it to `modelRuntime.providerConfig`.
2. Ensure all usages of `initModelRuntimeFromDB` are compatible with the enhanced return type (they can ignore `providerConfig` if not needed).
3. If `AiProviderModel` is still used elsewhere in `RuntimeExecutors.ts`, keep its import and only remove the now-unused instantiation and `findById` call; otherwise the import removal above is correct.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+130
to
+131
| const aiProviderModel = new AiProviderModel(ctx.serverDB, ctx.userId!); | ||
| const providerConfig = await aiProviderModel.findById(provider); |
Contributor
There was a problem hiding this comment.
suggestion (performance): Consider avoiding an extra DB round-trip for providerConfig on every execution.
This adds a second DB query via AiProviderModel.findById immediately after initModelRuntimeFromDB. If initModelRuntimeFromDB already loads (or can be extended to load) provider config, we could derive apiMode from that data instead and avoid the extra query on the hot path, reducing latency and DB load for frequent requests.
Suggested implementation:
import { type MessageModel } from '@/database/models/message';
import { type LobeChatDatabase } from '@/database/type';
import { initModelRuntimeFromDB } from '@/server/modules/ModelRuntime';
// 初始化 ModelRuntime (从数据库读取用户的 keyVaults)
const modelRuntime = await initModelRuntimeFromDB(ctx.serverDB, ctx.userId!, provider);
// Read user's provider config (already loaded by initModelRuntimeFromDB) to determine apiMode
// When user explicitly disables Responses API, set apiMode to 'chatCompletion'
const enableResponseApi = modelRuntime.providerConfig?.config?.enableResponseApi;
// Default to false for non-OpenAI providers, true only for OpenAI with explicit enable
const apiMode: 'responses' | 'chatCompletion' =
enableResponseApi === true ? 'responses' : 'chatCompletion';
// 构造 ChatStreamPayload
const chatPayload = {To fully implement this optimization, you also need to:
- Update
initModelRuntimeFromDB(insrc/server/modules/ModelRuntime.tsor its actual location) so that the returnedmodelRuntimeinstance includes the provider configuration:- Add a
providerConfig(or similarly named) property to theModelRuntimetype/interface. - Inside
initModelRuntimeFromDB, load the provider config (usingAiProviderModelor equivalent) and assign it tomodelRuntime.providerConfig.
- Add a
- Ensure all usages of
initModelRuntimeFromDBare compatible with the enhanced return type (they can ignoreproviderConfigif not needed). - If
AiProviderModelis still used elsewhere inRuntimeExecutors.ts, keep its import and only remove the now-unused instantiation andfindByIdcall; otherwise the import removal above is correct.
Contributor
|
Does this PR fix the issue: #10606
Original Content这个 pr 是否修复了该问题：https://github.com//pull/10606 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## next #11320 +/- ##
=========================================
Coverage 76.40% 76.40%
=========================================
Files 1134 1134
Lines 87549 87556 +7
Branches 11796 9854 -1942
=========================================
+ Hits 66892 66899 +7
Misses 20581 20581
Partials 76 76
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
…nt task execution Previously, server-side SubAgent task execution did not respect the user's 'Enable Responses API' setting from provider configuration. This caused issues when using providers like NewAPI with LiteLLM backend, where the Responses API format is not supported. The fix reads the user's provider config from the database and sets apiMode to 'chatCompletion' by default, only using 'responses' when the user has explicitly enabled it. This ensures SubAgent tasks use the same API format as regular chat.
- Add vi.mock for AiProviderModel in RuntimeExecutors tests to fix test failures - Fix misleading comment about OpenAI vs non-OpenAI provider distinction
tjx666
force-pushed
the
fix/subagent-response-api
branch
from
6aac83b to
cce7637
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Previously, server-side SubAgent task execution did not respect the user's 'Enable Responses API' setting from provider configuration. This caused issues when using providers like NewAPI with LiteLLM backend, where the Responses API format is not supported.
The fix reads the user's provider config from the database and sets apiMode to 'chatCompletion' by default, only using 'responses' when the user has explicitly enabled it. This ensures SubAgent tasks use the same API format as regular chat.
💻 Change Type
🔗 Related Issue
🔀 Description of Change
🧪 How to Test
📸 Screenshots / Videos
📝 Additional Information
Summary by Sourcery
Bug Fixes: