Skip to content

fix(hooks): preserve non-text parts in fromHookLLMRequest#23340

Closed
mk-imagine wants to merge 4 commits into
google-gemini:mainfrom
mk-imagine:fix/preserve-non-text-parts-in-hook-translator
Closed

fix(hooks): preserve non-text parts in fromHookLLMRequest#23340
mk-imagine wants to merge 4 commits into
google-gemini:mainfrom
mk-imagine:fix/preserve-non-text-parts-in-hook-translator

Conversation

@mk-imagine

@mk-imagine mk-imagine commented Mar 21, 2026

Copy link
Copy Markdown

Summary

fromHookLLMRequest creates new Content objects with only text parts, discarding all non-text parts (functionCall, functionResponse, inlineData, thought, etc.) from baseRequest. This breaks any BeforeModel hook that modifies text content in conversations containing tool calls — tool call/response history is silently destroyed, causing the model to lose context and loop.

Root cause

toHookLLMRequest intentionally filters Content entries to text-only messages (documented design for a simplified hook API). Some Content entries are skipped entirely (pure functionCall/functionResponse with no text), and others have their non-text parts stripped.

However, fromHookLLMRequest does not reverse this mapping — it replaces all contents with new text-only entries instead of merging text changes back into the original structure.

Reproduction

Any BeforeModel hook that modifies text + an agent loop with tool calls:

  1. User submits a prompt containing a string the hook wants to modify (e.g., PII blinding)
  2. BeforeModel hook receives the text-only messages, modifies text, returns modified llm_request
  3. fromHookLLMRequest creates new text-only contents, destroying functionCall/functionResponse parts
  4. Model never sees tool results → re-invokes the same tool → infinite loop

Queries that don't trigger text modification (hook returns {}) work correctly because the original SDK request with all parts is preserved.

Fix

When baseRequest is provided, fromHookLLMRequest now merges hook messages back into baseRequest.contents:

  1. Walk baseRequest.contents in order, maintaining a cursor into hookRequest.messages
  2. Content entries with text parts → consume next hook message, update text, preserve all non-text parts
  3. Content entries without text parts (e.g., pure functionCall) → preserve as-is in original position
  4. Extra hook messages beyond base contents → append as new text-only entries
  5. No baseRequest → fall back to current behavior (fully backwards-compatible)

Test plan

  • Round-trip with mixed content (text + functionCall parts) — non-text parts preserved
  • Text-only entries interleaved with function-only entries — correct cursor tracking
  • Hook modifies text in content that also has functionCall — text updated, functionCall untouched
  • Multiple text parts in one Content entry collapsed correctly (mirrors toHookLLMRequest's join)
  • baseRequest undefined → fallback to current behavior
  • baseRequest with no contents → fallback
  • Extra hook messages appended beyond base contents
  • All 10 existing tests continue to pass

@mk-imagine
fix(hooks): preserve non-text parts in fromHookLLMRequest
43b4d4f 
fromHookLLMRequest creates brand-new Content objects with only text parts,
discarding all non-text parts (functionCall, functionResponse, inlineData,
thought, etc.) from baseRequest. This breaks any BeforeModel hook that
modifies text content in conversations containing tool calls — the tool
call/response history is silently destroyed, causing the model to loop.

When baseRequest is provided, merge hook messages back into
baseRequest.contents instead of replacing them: update text parts from
hook messages, preserve non-text parts in their original positions, and
keep skipped entries (pure functionCall/functionResponse) intact. Falls
back to current text-only behavior when no baseRequest is provided.
@mk-imagine mk-imagine requested a review from a team as a code owner March 21, 2026 05:23
@google-cla

google-cla Bot commented Mar 21, 2026

Copy link
Copy Markdown

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@gemini-code-assist

gemini-code-assist Bot commented Mar 21, 2026

Copy link
Copy Markdown
Contributor

Summary of Changes

Hello, 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 delivers a crucial enhancement to the hookTranslator module, specifically addressing a defect in the fromHookLLMRequest function. The previous implementation inadvertently stripped out vital non-textual components, such as tool calls and responses, when processing modifications from BeforeModel hooks. This omission caused conversational AI models to lose critical context, often resulting in repetitive or erroneous behavior. The updated logic now intelligently merges textual changes from hooks while meticulously preserving all original non-text parts, thereby ensuring the integrity of the conversation history and preventing agentic loops.

Highlights

  • Preservation of Non-Text Parts: The fromHookLLMRequest function now correctly preserves non-textual content parts (like functionCall and functionResponse) when merging modified text from BeforeModel hooks back into the original LLM request. Previously, these non-text parts were discarded, leading to loss of context.
  • Prevention of Agent Looping: This fix resolves an issue where agents using BeforeModel hooks would enter infinite loops due to the model losing tool call/response history when the hook modified text content.
  • Intelligent Merging Logic: A new merging logic has been implemented in fromHookLLMRequest that iterates through the baseRequest.contents, updates text parts from the hookRequest.messages, and ensures all non-text parts are retained in their original positions. It also handles cases where new messages are added by the hook.
  • Backward Compatibility and Edge Cases: The updated function maintains backward compatibility by falling back to the previous text-only behavior if no baseRequest is provided or if it lacks contents. Comprehensive tests were added to cover various scenarios, including interleaved text and function-only entries, collapsing multiple text parts, and appending extra messages.
Using Gemini Code Assist

The 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 /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize 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 .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. 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

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

gemini-code-assist[bot]
gemini-code-assist Bot reviewed Mar 21, 2026
View reviewed changes

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request addresses a critical bug in fromHookLLMRequest that caused non-text parts of a request to be discarded after a BeforeModel hook modified text content, leading to potential infinite loops with tool calls. The fix intelligently merges the hook's text modifications back into the original request, preserving crucial non-text parts like functionCall and functionResponse. The implementation is robust, and the addition of a comprehensive test suite ensures the fix is effective and covers various edge cases. The changes are well-executed and I found no issues.

@gemini-cli gemini-cli Bot added the status/need-issue Pull requests that need to have an associated issue. label Mar 21, 2026
@github-actions github-actions Bot mentioned this pull request Mar 29, 2026
Open
@gemini-cli

gemini-cli Bot commented Apr 5, 2026

Copy link
Copy Markdown
Contributor

Hi there! Thank you for your interest in contributing to Gemini CLI.

To ensure we maintain high code quality and focus on our prioritized roadmap, we have updated our contribution policy (see Discussion #17383).

We only guarantee review and consideration of pull requests for issues that are explicitly labeled as 'help wanted'. All other community pull requests are subject to closure after 14 days if they do not align with our current focus areas. For this reason, we strongly recommend that contributors only submit pull requests against issues explicitly labeled as 'help-wanted'.

This pull request is being closed as it has been open for 14 days without a 'help wanted' designation. We encourage you to find and contribute to existing 'help wanted' issues in our backlog! Thank you for your understanding and for being part of our community!

@gemini-cli gemini-cli Bot closed this Apr 5, 2026
@mk-imagine mk-imagine mentioned this pull request Apr 16, 2026
Closed
@SandyTao520 SandyTao520 reopened this Apr 29, 2026
@SandyTao520 SandyTao520 self-requested a review April 29, 2026 18:14
@SandyTao520

SandyTao520 commented Apr 29, 2026

Copy link
Copy Markdown
Contributor

LGTM, can you merge main and adapt to the latest fromHookLLMRequest format?

@gemini-cli

gemini-cli Bot commented Apr 30, 2026

Copy link
Copy Markdown
Contributor

Hi there! Thank you for your interest in contributing to Gemini CLI.

To ensure we maintain high code quality and focus on our prioritized roadmap, we have updated our contribution policy (see Discussion #17383).

We only guarantee review and consideration of pull requests for issues that are explicitly labeled as 'help wanted'. All other community pull requests are subject to closure after 14 days if they do not align with our current focus areas. For this reason, we strongly recommend that contributors only submit pull requests against issues explicitly labeled as 'help-wanted'.

This pull request is being closed as it has been open for 14 days without a 'help wanted' designation. We encourage you to find and contribute to existing 'help wanted' issues in our backlog! Thank you for your understanding and for being part of our community!

@gemini-cli gemini-cli Bot closed this Apr 30, 2026
@SandyTao520 SandyTao520 mentioned this pull request Apr 30, 2026
Merged
18 tasks
@sripasg sripasg added the size/l A large sized PR label Jun 2, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@SandyTao520 SandyTao520 Awaiting requested review from SandyTao520

1 more reviewer

@gemini-code-assist gemini-code-assist[bot] gemini-code-assist[bot] left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

size/l A large sized PR status/need-issue Pull requests that need to have an associated issue.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@mk-imagine @SandyTao520 @sripasg