fix: detect Google-format tool messages in truncation grouping#552
Open
mathebrito wants to merge 2 commits intoplastic-labs:mainfrom
Open
fix: detect Google-format tool messages in truncation grouping#552mathebrito wants to merge 2 commits intoplastic-labs:mainfrom
mathebrito wants to merge 2 commits intoplastic-labs:mainfrom
Conversation
…nt to internal disk
_is_tool_use_message() and _is_tool_result_message() only detected Anthropic (content[].type=tool_use/tool_result) and OpenAI (tool_calls field / role=tool) formats. Google-format messages using parts[].function_call and parts[].function_response were treated as regular messages by _group_into_units(). When truncate_messages_to_fit() ran during the tool-calling loop with Google provider, it could remove one half of a function_call → function_response pair, producing an invalid conversation sequence. Gemini then returned 400 INVALID_ARGUMENT. Add Google format detection to both helper functions so truncation preserves function_call/function_response pairs as atomic units. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
WalkthroughAdded PostgreSQL service with pgvector support to local Docker Compose configuration. Enhanced tool-call and tool-result detection in client utilities to recognize Google/Gemini message formats alongside existing Anthropic/OpenAI checks. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
docker-compose.local.yml (1)
10-10: Avoid committing fixed DB passwords, even in local compose.Use env interpolation (
${POSTGRES_PASSWORD:-...}) so local secrets can be overridden safely.Suggested secret handling patch
- - POSTGRES_PASSWORD=honcho + - POSTGRES_PASSWORD=${POSTGRES_PASSWORD:-postgres}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docker-compose.local.yml` at line 10, The compose file currently hardcodes POSTGRES_PASSWORD=honcho which exposes a fixed local secret; change the environment entry for POSTGRES_PASSWORD in docker-compose.local.yml to use env interpolation (e.g. POSTGRES_PASSWORD=${POSTGRES_PASSWORD:-honcho}) so local runs default to the existing value but callers can override via environment or an .env file; update the POSTGRES_PASSWORD line only and do not add the literal secret elsewhere in the repo.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docker-compose.local.yml`:
- Around line 6-10: The docker-compose Postgres credentials/port
(POSTGRES_DB=honcho, POSTGRES_USER=honcho, POSTGRES_PASSWORD=honcho, and host
port 5433) do not match the runtime defaults expected by DB_CONNECTION_URI
(postgres:postgres@localhost:5432/postgres); either change the docker-compose
service to use POSTGRES_DB=postgres, POSTGRES_USER=postgres,
POSTGRES_PASSWORD=postgres and map "127.0.0.1:5432:5432" to match the runtime
defaults, or explicitly set DB_CONNECTION_URI in the service environment to
"postgresql://honcho:honcho@localhost:5433/honcho" (and update .env.template/any
config default references accordingly) so the app can connect without overriding
at runtime.
- Around line 23-26: The host-specific bind mount under driver_opts (device:
/Users/headless/docker-volumes/honcho-postgres) is non-portable; replace it with
a named volume or an env-substituted host path: update the driver_opts/device
entry to reference either a Docker named volume (create a volumes: entry and use
that name) or use an environment variable like ${HONCHO_PG_HOST_PATH} and
document it in .env, ensuring the docker-compose service uses the new volume
name instead of the hardcoded path.
---
Nitpick comments:
In `@docker-compose.local.yml`:
- Line 10: The compose file currently hardcodes POSTGRES_PASSWORD=honcho which
exposes a fixed local secret; change the environment entry for POSTGRES_PASSWORD
in docker-compose.local.yml to use env interpolation (e.g.
POSTGRES_PASSWORD=${POSTGRES_PASSWORD:-honcho}) so local runs default to the
existing value but callers can override via environment or an .env file; update
the POSTGRES_PASSWORD line only and do not add the literal secret elsewhere in
the repo.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 04227ef9-04ef-4aa4-b0d9-43f160fb0301
📒 Files selected for processing (2)
docker-compose.local.ymlsrc/utils/clients.py
Comment on lines
+6
to
+10
| - "127.0.0.1:5433:5432" | ||
| environment: | ||
| - POSTGRES_DB=honcho | ||
| - POSTGRES_USER=honcho | ||
| - POSTGRES_PASSWORD=honcho |
Contributor
There was a problem hiding this comment.
Configuration mismatch will break default local startup.
Lines 6-10 configure Postgres as honcho:honcho@localhost:5433/honcho, but runtime defaults expect postgres:postgres@localhost:5432/postgres (see src/config.py and .env.template). Without explicit DB_CONNECTION_URI override, DB init/connection will fail.
Suggested alignment patch
- - "127.0.0.1:5433:5432"
+ - "127.0.0.1:5432:5432"
@@
- - POSTGRES_DB=honcho
- - POSTGRES_USER=honcho
- - POSTGRES_PASSWORD=honcho
+ - POSTGRES_DB=postgres
+ - POSTGRES_USER=postgres
+ - POSTGRES_PASSWORD=postgres📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| - "127.0.0.1:5433:5432" | |
| environment: | |
| - POSTGRES_DB=honcho | |
| - POSTGRES_USER=honcho | |
| - POSTGRES_PASSWORD=honcho | |
| - "127.0.0.1:5432:5432" | |
| environment: | |
| - POSTGRES_DB=postgres | |
| - POSTGRES_USER=postgres | |
| - POSTGRES_PASSWORD=postgres |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docker-compose.local.yml` around lines 6 - 10, The docker-compose Postgres
credentials/port (POSTGRES_DB=honcho, POSTGRES_USER=honcho,
POSTGRES_PASSWORD=honcho, and host port 5433) do not match the runtime defaults
expected by DB_CONNECTION_URI (postgres:postgres@localhost:5432/postgres);
either change the docker-compose service to use POSTGRES_DB=postgres,
POSTGRES_USER=postgres, POSTGRES_PASSWORD=postgres and map "127.0.0.1:5432:5432"
to match the runtime defaults, or explicitly set DB_CONNECTION_URI in the
service environment to "postgresql://honcho:honcho@localhost:5433/honcho" (and
update .env.template/any config default references accordingly) so the app can
connect without overriding at runtime.
Comment on lines
+23
to
+26
| driver_opts: | ||
| type: none | ||
| o: bind | ||
| device: /Users/headless/docker-volumes/honcho-postgres |
Contributor
There was a problem hiding this comment.
Host-specific bind mount path is non-portable.
Line 26 hardcodes /Users/headless/..., which will fail on other machines/OSes. Prefer a plain named volume (or env-substituted host path if bind-mount is required).
Suggested portability patch
volumes:
honcho-postgres-data:
- driver: local
- driver_opts:
- type: none
- o: bind
- device: /Users/headless/docker-volumes/honcho-postgres
+ driver: local🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docker-compose.local.yml` around lines 23 - 26, The host-specific bind mount
under driver_opts (device: /Users/headless/docker-volumes/honcho-postgres) is
non-portable; replace it with a named volume or an env-substituted host path:
update the driver_opts/device entry to reference either a Docker named volume
(create a volumes: entry and use that name) or use an environment variable like
${HONCHO_PG_HOST_PATH} and document it in .env, ensuring the docker-compose
service uses the new volume name instead of the hardcoded path.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Summary
_is_tool_use_message()and_is_tool_result_message()insrc/utils/clients.pyonly detected Anthropic (content[].type=tool_use/tool_result) and OpenAI (tool_callsfield /role=tool) formatsparts[].function_callandparts[].function_responsewere treated as regular standalone messages by_group_into_units()truncate_messages_to_fit()ran during the tool-calling loop with Google provider, it could independently remove one half of afunction_call→function_responsepair, producing an invalid conversation sequence400 INVALID_ARGUMENT: "Please ensure that function call turn comes immediately after a user turn or after a function response turn"Fix
Add Google format detection (
parts[].function_call/parts[].function_response) to both_is_tool_use_message()and_is_tool_result_message(), so_group_into_units()keeps function call/response pairs as atomic units during truncation.Reproduction
Triggered consistently when:
gemini-3-flash-preview) for dialectic queriesMAX_INPUT_TOKENS(32,768)Test plan
targetparameter) now succeed🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Chores