Repository navigation
Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses an issue where binary data injection for audio and video files failed for Gemini 3 models. By ensuring that the synthetic binary acknowledgment is treated as a plain-text model turn rather than a thought-only part, the media content is correctly separated into its own user turn. This change maintains compatibility with the Code Assist API requirements and prevents invalid request shapes. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
|
📊 PR Size: size/M
|
![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 ensures that synthetic binary acknowledgment turns are preserved as plain text model turns instead of being stripped or coalesced during history hardening. This prevents injected binary data from coalescing into a functionResponse turn, which avoids 400 errors from the Code Assist API. Relevant tests have been added and updated to verify this behavior. I have no feedback to provide as there are no review comments.
dbac79c to
f0db098
Compare
gemini-cli
Bot
added
area/agent
…s in its own turn Audio and video read by read_file are injected as user[functionResponse] -> model[ack] -> user[inlineData]. The ack is a thought-only part, so stripThoughts, scrubHistory and scrubContents drop it and the two user turns coalesce into user[functionResponse, inlineData]. The Code Assist API rejects that shape with 400 "Requests ending with a model turn are not supported". Send the ack as a plain-text model turn instead. The recorded history is unchanged, so sessions recorded with the thought ack are fixed on resume. Fixes google-gemini#30229 Claude-Session: https://claude.ai/code/session_01Sw78yR9Us3cqRudv2DDgdA
f0db098 to
47f1eea
Compare
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request updates the history hardening and thought stripping logic to preserve synthetic binary acknowledgment turns as plain text model turns, preventing injected media from coalescing into function response turns. One review comment was provided, suggesting a defensive check for undefined array elements and explicit handling of optional properties in geminiChat.ts to prevent potential runtime errors.
Note: Security Review did not run due to the size of the PR.
| if (stripped.at(-1)?.role === 'user') { | ||
| for (let i = stripped.length - 1; i >= 0; i--) { | ||
| const prev = stripped[i]; | ||
| if (prev.role === 'user') { | ||
| if (prev.parts?.some((part) => !!part.functionResponse)) { | ||
| hasToolResponseUserTurn = true; | ||
| break; | ||
| } | ||
| continue; | ||
| } | ||
| if (isBinaryAckTurn(prev)) { | ||
| continue; | ||
| } | ||
| break; | ||
| } | ||
| } |
There was a problem hiding this comment.
The loop accesses prev.role and prev.parts directly without checking if prev is defined. While stripped is populated sequentially, adding a defensive check for prev prevents potential runtime crashes or compilation errors under strict TypeScript configurations (such as noUncheckedIndexedAccess). Additionally, since parts is an optional property, we should handle the undefined case explicitly using the nullish coalescing operator (??) to align with our repository guidelines.
if (stripped.at(-1)?.role === 'user') {
for (let i = stripped.length - 1; i >= 0; i--) {
const prev = stripped[i];
if (!prev) {
break;
}
if (prev.role === 'user') {
if (prev.parts?.some((part) => !!part.functionResponse) ?? false) {
hasToolResponseUserTurn = true;
break;
}
continue;
}
if (isBinaryAckTurn(prev)) {
continue;
}
break;
}
}References
- When consuming an object, if a property is optional in its type definition (interface), callers must handle the
undefinedcase (e.g., by providing a default with??).
Summary
With Gemini 3 models,
read_fileon audio or video fails with 400 "Requests ending with a model turn are not supported" when using Code Assist (Login with Google). The binary injection from #26352 sends the media in its own user turn behind a synthetic model ack. Because that ack is a thought-only part, thought stripping drops it. The media then coalesces into thefunctionResponseturn, which is the shape the API rejects. This PR keeps the ack as a plain-text model turn when history is sent.Details
stripThoughts(both copies),scrubHistoryandscrubContents, a synthetic binary ack turn becomesmodel[text: "Binary content received. Proceeding with analysis."]instead of being dropped. The!isSyntheticBinaryAckTurnexclusions are removed because those branches can no longer be reached for the ack.historyHardeningtest that expected the merged[functionResponse, inlineData]turn now expects the three-turn shape. A newGeminiChattest checks the request a modern model receives. Both new assertions fail onmainand pass with this change.functionResponse.parts.Related Issues
Fixes #30229
How to Validate
Unit tests:
npm test -w @google/gemini-cli-core -- src/utils/historyHardening.test.ts src/core/geminiChat.test.tsLive check on Linux with Login with Google,
-m gemini-3.8-flash, and a small.mp4and.wavin the workspace. I recorded the request shapes with a local proxy viaCODE_ASSIST_ENDPOINT:e5d88fb2cuser[functionResponse(read_file), inlineData(video/mp4)]user[functionResponse],model[text],user[inlineData(video/mp4)]e5d88fb2cuser[functionResponse(read_file), inlineData(audio/wav)]user[functionResponse],model[text],user[inlineData(audio/wav)]Note: on
main, an audio or video file larger than about 48 KB also fails with "Base64 decoding failed". Since #30180,truncateFunctionResponseParttruncates the injected base64 at 64 KB. That is a separate issue, so the live check used small files.Pre-Merge Checklist
https://claude.ai/code/session_01Sw78yR9Us3cqRudv2DDgdA