Skip to content

fix: empty model output error may misfire when use gemini #7377

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

Merged
Soulter merged 1 commit into from
Apr 5, 2026
Merged

fix: empty model output error may misfire when use gemini #7377

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
37 changes: 24 additions & 13 deletions astrbot/core/provider/sources/gemini_source.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -461,14 +461,19 @@ def _process_content_parts(
self,
candidate: types.Candidate,
llm_response: LLMResponse,
*,
validate_output: bool = True,
) -> MessageChain:
"""处理内容部分并构建消息链"""
if not candidate.content:
logger.warning(f"收到的 candidate.content 为空: {candidate}")
raise EmptyModelOutputError(
"Gemini candidate content is empty. "
f"finish_reason={candidate.finish_reason}"
)
if validate_output:
raise EmptyModelOutputError(
"Gemini candidate content is empty. "
f"finish_reason={candidate.finish_reason}"
)
llm_response.result_chain = MessageChain(chain=[])
Comment on lines +470 to +475
Copy link
Copy Markdown
Contributor

@sourcery-ai sourcery-ai bot Apr 5, 2026

Choose a reason for hiding this comment

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

issue (bug_risk): Returning an empty MessageChain when candidate.content is empty may mask streaming heartbeats/keep-alive chunks

In the streaming path, _process_content_parts(..., validate_output=False) will hit this branch for chunks where candidate.content is empty (e.g., heartbeats or tool-only updates). That now resets llm_response.result_chain to an empty MessageChain, discarding previously accumulated tokens. Before, this would raise and stop processing, making the issue visible. For streaming, it would be safer to treat empty candidate.content as a no-op (return the existing result_chain unchanged) to avoid losing prior content.

return llm_response.result_chain
Comment on lines +470 to +476
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist bot Apr 5, 2026

Choose a reason for hiding this comment

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

medium

The early return (line 476) and the potential exception (line 471) occur before the safety and policy checks (lines 481-494). When Gemini blocks a response due to safety filters, it often returns a candidate with no content. In such cases, this function will either raise a generic EmptyModelOutputError or return an empty chain, bypassing the specific safety error messages. This leads to the "misfire" mentioned in the PR title, where the user sees an empty output error instead of a safety violation warning.

Consider moving the finish_reason validation logic (the safety and policy checks) to the very beginning of the function so they are executed even if candidate.content is missing.


finish_reason = candidate.finish_reason
result_parts: list[types.Part] | None = candidate.content.parts
Expand All @@ -490,10 +495,13 @@ def _process_content_parts(

if not result_parts:
logger.warning(f"收到的 candidate.content.parts 为空: {candidate}")
raise EmptyModelOutputError(
"Gemini candidate content parts are empty. "
f"finish_reason={candidate.finish_reason}"
)
if validate_output:
raise EmptyModelOutputError(
"Gemini candidate content parts are empty. "
f"finish_reason={candidate.finish_reason}"
)
llm_response.result_chain = MessageChain(chain=[])
return llm_response.result_chain

# 提取 reasoning content
reasoning = self._extract_reasoning_content(candidate)
Expand Down Expand Up @@ -550,11 +558,12 @@ def _process_content_parts(
llm_response.reasoning_signature = base64.b64encode(ts).decode("utf-8")
chain_result = MessageChain(chain=chain)
llm_response.result_chain = chain_result
self._ensure_usable_response(
llm_response,
response_id=None,
finish_reason=str(finish_reason) if finish_reason is not None else None,
)
if validate_output:
self._ensure_usable_response(
llm_response,
response_id=None,
finish_reason=str(finish_reason) if finish_reason is not None else None,
)
return chain_result

async def _query(self, payloads: dict, tools: ToolSet | None) -> LLMResponse:
Expand Down Expand Up @@ -708,6 +717,7 @@ async def _query_stream(
llm_response.result_chain = self._process_content_parts(
chunk.candidates[0],
llm_response,
validate_output=False,
Comment on lines 717 to +720
Copy link
Copy Markdown
Contributor

@sourcery-ai sourcery-ai bot Apr 5, 2026

Choose a reason for hiding this comment

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

suggestion (bug_risk): Streaming path now treats all chunks as non-validating, including the final one

Because both the streaming branch and the last(...) branch pass validate_output=False, the fully assembled final_response never gets the stricter validation. To allow leniency for partial/empty chunks but still enforce that the final response is valid, consider using validate_output=False only for intermediate chunks and validate_output=True for the final chunk, or run a separate final validation after aggregation.

Suggested implementation:

                llm_response.result_chain = self._process_content_parts(
                    chunk.candidates[0],
                    llm_response,
                    validate_output=False,
                )
                llm_response.id = chunk.response_id
                if chunk.usage_metadata:
                    # For the aggregated final_response, enable validation so that
                    # the fully-assembled output is checked strictly.
                    final_response.result_chain = self._process_content_parts(
                        chunk.candidates[0],
                        final_response,
                        validate_output=True,
                    )
                    final_response.id = chunk.response_id
                    if chunk.usage_metadata:

To fully meet your comment (validating only the final aggregated response, not every chunk):

  1. Identify where the streaming loop over chunks terminates and where final_response is returned.

  2. Move or add a single validation call there, e.g.:

    self._ensure_usable_response(
    final_response,
    response_id=final_response.id,
    finish_reason=str(final_finish_reason) if final_finish_reason is not None else None,
)

  3. If you do add this post-loop validation, you can consider reverting validate_output=True above to the default (omit the argument) or even leave it False again, relying solely on the final _ensure_usable_response call to enforce strict validation on the aggregated response.

)
llm_response.id = chunk.response_id
if chunk.usage_metadata:
Expand Down Expand Up @@ -738,6 +748,7 @@ async def _query_stream(
final_response.result_chain = self._process_content_parts(
chunk.candidates[0],
final_response,
validate_output=False,
)
Comment on lines 748 to 752
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist bot Apr 5, 2026

Choose a reason for hiding this comment

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

medium

In _query_stream, if the final chunk has a finish_reason but no content.parts, the block starting at line 745 (in the full file) is skipped. Consequently, final_response is initialized later at line 760 without capturing the response_id and usage_metadata from that final chunk. This results in missing metadata for the complete response.

Consider extracting the metadata (id and usage) from the final chunk regardless of whether it contains new content parts.

final_response.id = chunk.response_id
if chunk.usage_metadata:
Expand Down
Loading