fix(core): 将空响应视为失败并交给模型重试#809
Conversation
Walkthrough将流式与非流式响应验证从仅检测是否收到数据块（hasChunk）改为检测是否有实际响应内容或工具/函数调用证据（hasResponse）；调整重试回退为指数退避，并扩展工具调用检测类型与相关判断位置。 Changes
Sequence Diagram(s)（无） Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
诗歌
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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.
🧹 Nitpick comments (2)
packages/core/src/llm-core/platform/model.ts (2)
330-337: 类型签名扩展合理，需修复格式将参数类型从
AIMessageChunk扩展为AIMessage | AIMessageChunk是合理的，因为现在需要在非流式路径（第 507-509 行）中复用此方法。对tool_call_chunks的类型转换是必要的，因为只有AIMessageChunk有此属性。🛠️ 修复 Prettier 格式
private _hasToolCallChunk(message?: AIMessage | AIMessageChunk): boolean { return ( (message?.tool_calls?.length ?? 0) > 0 || - ((message as AIMessageChunk | undefined)?.tool_call_chunks?.length ?? - 0) > 0 || + ((message as AIMessageChunk | undefined)?.tool_call_chunks + ?.length ?? 0) > 0 || (message?.invalid_tool_calls?.length ?? 0) > 0 ) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/llm-core/platform/model.ts` around lines 330 - 337, The _hasToolCallChunk method signature change to accept AIMessage | AIMessageChunk is fine; fix the formatting so it meets Prettier/style rules: ensure the conditional expression is line-broken and indented cleanly, keep the explicit cast for accessing tool_call_chunks on AIMessageChunk (e.g., (message as AIMessageChunk | undefined)?.tool_call_chunks) and align the nullish coalescing checks for tool_calls, tool_call_chunks, and invalid_tool_calls so the parentheses and operators are on the correct lines to satisfy the project's formatter.
256-260: 代码逻辑正确，但有格式问题
hasResponse的追踪逻辑正确地区分了"收到数据块"和"收到有效内容"。静态分析工具指出此处有格式问题。🛠️ 修复 Prettier 格式
hasResponse = hasResponse || hasTool || - getMessageContent(chunk.message.content).trim().length > 0 + getMessageContent(chunk.message.content).trim() + .length > 0🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/llm-core/platform/model.ts` around lines 256 - 260, 代码逻辑无误但存在 Prettier 格式问题：在包含 hasResponse 更新和 yield chunk 的代码块（涉及变量 hasResponse、hasTool、getMessageContent 和 chunk）应用项目的 Prettier/格式化规则以修复换行与缩进，使表达式和 yield 语句符合统一风格（例如将条件表达式和 yield 分为单独行并确保正确空格/分号），然后保存以通过静态检查。
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/core/src/llm-core/platform/model.ts`:
- Around line 330-337: The _hasToolCallChunk method signature change to accept
AIMessage | AIMessageChunk is fine; fix the formatting so it meets
Prettier/style rules: ensure the conditional expression is line-broken and
indented cleanly, keep the explicit cast for accessing tool_call_chunks on
AIMessageChunk (e.g., (message as AIMessageChunk | undefined)?.tool_call_chunks)
and align the nullish coalescing checks for tool_calls, tool_call_chunks, and
invalid_tool_calls so the parentheses and operators are on the correct lines to
satisfy the project's formatter.
- Around line 256-260: 代码逻辑无误但存在 Prettier 格式问题：在包含 hasResponse 更新和 yield chunk
的代码块（涉及变量 hasResponse、hasTool、getMessageContent 和 chunk）应用项目的
Prettier/格式化规则以修复换行与缩进，使表达式和 yield 语句符合统一风格（例如将条件表达式和 yield
分为单独行并确保正确空格/分号），然后保存以通过静态检查。
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: fa60dab4-877f-439b-b914-d78f7e70a20d
📒 Files selected for processing (1)
packages/core/src/llm-core/platform/model.ts
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request improves response validation in ChatLunaChatModel by introducing a hasResponse flag to track whether a stream or completion contains actual content or tool calls, ensuring that empty responses are correctly identified as failures. Feedback suggests optimizing the streaming loop to avoid redundant string processing once a response is detected and notes that the final validation check is redundant for streaming paths already handled by internal assertions.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/core/src/llm-core/platform/model.ts (1)
516-524: 非流式路径的空响应验证对非流式响应的
_hasResponse检查是正确且必要的。对于流式响应，虽然_streamResponseChunks已在内部进行了检查，但此处作为安全兜底也是合理的。语义方面的小建议：根据相关代码片段，
API_REQUEST_FAILED(103) 在错误码定义中语义上指代 API/传输层失败。此处用于"响应为空"可能导致上游错误处理代码混淆（例如区分网络故障与模型返回空内容）。如果未来需要更精细的错误区分，可考虑新增专用错误码如EMPTY_RESPONSE。但当前实现可有效复用现有重试机制，满足 PR 需求。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/llm-core/platform/model.ts` around lines 516 - 524, The non-stream empty-response check inside model.ts currently throws ChatLunaError with ChatLunaErrorCode.API_REQUEST_FAILED which conflates transport failures with empty model output; create a new enum value ChatLunaErrorCode.EMPTY_RESPONSE (or similar) and throw ChatLunaError(ChatLunaErrorCode.EMPTY_RESPONSE) in the branch where !_hasResponse(response.message ...) instead of API_REQUEST_FAILED, leaving _streamResponseChunks and _hasResponse logic intact; update any error constructors or handlers as needed to recognize the new EMPTY_RESPONSE code.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/core/src/llm-core/platform/model.ts`:
- Around line 516-524: The non-stream empty-response check inside model.ts
currently throws ChatLunaError with ChatLunaErrorCode.API_REQUEST_FAILED which
conflates transport failures with empty model output; create a new enum value
ChatLunaErrorCode.EMPTY_RESPONSE (or similar) and throw
ChatLunaError(ChatLunaErrorCode.EMPTY_RESPONSE) in the branch where
!_hasResponse(response.message ...) instead of API_REQUEST_FAILED, leaving
_streamResponseChunks and _hasResponse logic intact; update any error
constructors or handlers as needed to recognize the new EMPTY_RESPONSE code.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 25f7b321-ab35-4c55-802a-840ffc5e5f91
📒 Files selected for processing (1)
packages/core/src/llm-core/platform/model.ts
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/core/src/llm-core/platform/model.ts (1)
345-357:_hasResponse检测逻辑全面，有一处可选优化。方法覆盖了多种响应来源：内容、工具调用块、以及
additional_kwargs中的遗留格式。根据相关代码片段，适配器（shared-adapter、gemini、claude）将工具调用放在tool_call_chunks而非additional_kwargs.tool_calls。第 353-354 行对
additional_kwargs.tool_calls的检查可能冗余，因为_hasToolCallChunk已经检查了tool_call_chunks。但作为防御性代码处理原始 API 响应的边缘情况，保留也是合理的。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/llm-core/platform/model.ts` around lines 345 - 357, The _hasResponse method currently checks content, _hasToolCallChunk, additional_kwargs.tool_calls, and additional_kwargs.function_call; to optimize and clarify intent, reorder the logic to call this._hasToolCallChunk(message) first (since adapters put tool calls in tool_call_chunks) and keep the additional_kwargs.tool_calls check only as a defensive fallback; ensure you still null-check and cast (as unknown[]|undefined) when inspecting message?.additional_kwargs?.tool_calls and preserve the function_call check so behavior doesn't change, referencing the _hasResponse and _hasToolCallChunk symbols.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/core/src/llm-core/platform/model.ts`:
- Around line 345-357: The _hasResponse method currently checks content,
_hasToolCallChunk, additional_kwargs.tool_calls, and
additional_kwargs.function_call; to optimize and clarify intent, reorder the
logic to call this._hasToolCallChunk(message) first (since adapters put tool
calls in tool_call_chunks) and keep the additional_kwargs.tool_calls check only
as a defensive fallback; ensure you still null-check and cast (as
unknown[]|undefined) when inspecting message?.additional_kwargs?.tool_calls and
preserve the function_call check so behavior doesn't change, referencing the
_hasResponse and _hasToolCallChunk symbols.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: b144baf8-3167-4f23-bdb8-30c398e6f76b
📒 Files selected for processing (1)
packages/core/src/llm-core/platform/model.ts
2 participants
变更说明
API_REQUEST_FAILED，交给模型层现有maxRetries处理。验证
yarn fast-build core