feat(hermes): capture tool calls individually and remove content truncation in sync_turn#1012
feat(hermes): capture tool calls individually and remove content truncation in sync_turn#1012ggoldani wants to merge 2 commits into
Conversation
…cation in sync_turn The Hermes integration's sync_turn had two issues that degraded memory quality for Hermes Agent users: 1. Content truncation — tool_input was hard-cut to 500 chars and tool_output to 2000 chars, losing long prompts, code blocks, and tool results. The agentmemory server already truncates to 8000 chars server-side, so the plugin-side limits were needlessly aggressive. 2. No per-tool-call capture — Hermes passes the full OpenAI-format messages list (including assistant tool_calls and matching tool results) via the messages kwarg, but sync_turn ignored it entirely. Every turn produced a single conversation observation with concatenated text, losing granular tool-call data. Changes: - Remove [:500]/[:2000] truncation - Add _extract_tool_observations() helper that walks the messages list, matches assistant tool_calls to their tool results by tool_call_id, and emits one observation per call/result pair - Cap at 10 most-recent tool calls per turn to avoid observation flooding - Keep the conversation observation as a fallback (backward compatible) Tested: 9 new assertions in hermes-plugin.test.ts, E2E validated against live agentmemory server.
|
@ggoldani is attempting to deploy a commit to the rohitg00's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds a Hermes helper that derives per-tool observations from ChangesHermes sync_turn rich capture
Estimated code review effort: 2 (Simple) | ~15 minutes Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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.
Actionable comments posted: 1
🧹 Nitpick comments (1)
test/hermes-plugin.test.ts (1)
73-104: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftRegex-on-source assertions don't verify actual runtime behavior.
These tests only check that certain string patterns exist/don't exist in the Python source; they don't exercise
_extract_tool_observationswith real message payloads, so they can't catch logic bugs (e.g., matching order, id-collision handling,max_resultsactually capping the returned list). This follows the existing style in this file (e.g.readAgentMemoryProviderHookMethods), but consider adding behavioral tests (e.g., via a Python subprocess harness or pytest) that call the helper directly with samplemessagesand assert on the returned observation list.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/hermes-plugin.test.ts` around lines 73 - 104, The current checks in hermes-plugin.test.ts only inspect source text and do not validate _extract_tool_observations behavior at runtime. Replace or supplement the regex assertions with behavioral tests that execute the Python helper with real messages payloads, verifying tool_call_id matching, fallback conversation handling, and that max_results actually limits the returned observations. Use the existing sync_turn / _extract_tool_observations symbols to locate the code under test.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@integrations/hermes/__init__.py`:
- Around line 208-218: The pending tool-call tracking in the tool-call parsing
block can collide when call.get("id", "") is empty, causing multiple calls to
overwrite the same entry and drop earlier observations. Update the logic around
the call_id assignment and pending[...] insertion to ensure every tool call gets
a unique key even when the Hermes payload omits an id, and keep the existing
tool_name/tool_input mapping intact while preventing empty-string collisions.
---
Nitpick comments:
In `@test/hermes-plugin.test.ts`:
- Around line 73-104: The current checks in hermes-plugin.test.ts only inspect
source text and do not validate _extract_tool_observations behavior at runtime.
Replace or supplement the regex assertions with behavioral tests that execute
the Python helper with real messages payloads, verifying tool_call_id matching,
fallback conversation handling, and that max_results actually limits the
returned observations. Use the existing sync_turn / _extract_tool_observations
symbols to locate the code under test.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8349cdda-622d-4283-a91f-0495288820d6
📒 Files selected for processing (2)
integrations/hermes/__init__.pytest/hermes-plugin.test.ts
| call_id = call.get("id", "") | ||
| try: | ||
| args_str = fn.get("arguments", "") | ||
| if isinstance(args_str, dict): | ||
| args_str = json.dumps(args_str) | ||
| except (TypeError, ValueError): | ||
| args_str = str(fn.get("arguments", "")) | ||
| pending[call_id] = { | ||
| "tool_name": name, | ||
| "tool_input": args_str, | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Missing tool_call_id collisions can silently drop observations.
When call.get("id", "") is empty/missing, multiple tool calls in the same or different assistant messages will all key into pending under "", so an earlier unmatched call gets silently overwritten before it can be paired with its result.
🐛 Proposed fix to avoid collisions on missing ids
call_id = call.get("id", "")
+ if not call_id:
+ call_id = f"__noid_{len(pending)}_{name}"
try:📝 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.
| call_id = call.get("id", "") | |
| try: | |
| args_str = fn.get("arguments", "") | |
| if isinstance(args_str, dict): | |
| args_str = json.dumps(args_str) | |
| except (TypeError, ValueError): | |
| args_str = str(fn.get("arguments", "")) | |
| pending[call_id] = { | |
| "tool_name": name, | |
| "tool_input": args_str, | |
| } | |
| call_id = call.get("id", "") | |
| if not call_id: | |
| call_id = f"__noid_{len(pending)}_{name}" | |
| try: | |
| args_str = fn.get("arguments", "") | |
| if isinstance(args_str, dict): | |
| args_str = json.dumps(args_str) | |
| except (TypeError, ValueError): | |
| args_str = str(fn.get("arguments", "")) | |
| pending[call_id] = { | |
| "tool_name": name, | |
| "tool_input": args_str, | |
| } |
🧰 Tools
🪛 ast-grep (0.44.1)
[info] 211-211: use jsonify instead of json.dumps for JSON output
Context: json.dumps(args_str)
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@integrations/hermes/__init__.py` around lines 208 - 218, The pending
tool-call tracking in the tool-call parsing block can collide when
call.get("id", "") is empty, causing multiple calls to overwrite the same entry
and drop earlier observations. Update the logic around the call_id assignment
and pending[...] insertion to ensure every tool call gets a unique key even when
the Hermes payload omits an id, and keep the existing tool_name/tool_input
mapping intact while preventing empty-string collisions.
CodeRabbit review caught that empty call_id ('') causes all id-less tool
calls to overwrite the same pending[''] entry, dropping earlier calls.
Skip calls without an id — the conversation fallback observation already
captures the full turn when tool calls can't be matched to results.
1 participant
Summary
The Hermes integration's
sync_turnhad two issues that degraded memory quality for Hermes Agent users:Content truncation —
tool_inputwas hard-cut to 500 chars andtool_outputto 2000 chars, losing long prompts, code blocks, and tool results. The agentmemory server already truncates to 8000 chars server-side, so the plugin-side limits were needlessly aggressive.No per-tool-call capture — Hermes passes the full OpenAI-format
messageslist (including assistanttool_callsand matchingtoolresults) via themessageskwarg, butsync_turnignored it entirely. Every turn produced a single "conversation" observation with concatenated text, losing granular tool-call data.Changes
[:500]/[:2000]truncation — let the server handle size limits_extract_tool_observations()helper — walks themessageslist, matches assistanttool_callsto theirtoolresults bytool_call_id, and emits one observation per call/result pairBehavior
tool_inputcut at 500 charstool_outputcut at 2000 charsmessageskwarg ignoredBackward compatibility
messageskwarg → identical (conversation-only)messagesbut no tool calls → identical (conversation-only)messagesand tool calls → conversation + individual tool-call obsTest plan
test/hermes-plugin.test.tsSummary by CodeRabbit
New Features
Bug Fixes