revert: remove unnecessary prompt and parser changes from #156#169
Merged
graycyrus merged 1 commit intotinyhumansai:mainfrom Apr 1, 2026
Merged
Conversation
…i#156 The actual fix in tinyhumansai#156 was adding chat(ChatRequest) to ReliableProvider. The prompt changes in instructions.rs and the bracket tool call parser in parse.rs were added during investigation but are not needed — the model uses native tool calls when ReliableProvider properly delegates to the inner provider.
Contributor
📝 WalkthroughWalkthroughThe pull request simplifies agent loop instructions by consolidating multiple anti-hallucination constraints into a single critical requirement and updating example guidance. Concurrently, bracket-style tool-call parsing logic and related open-tag entries are removed from the parser, with the fallback behavior simplified to emit warnings rather than attempt bracket pseudo-syntax extraction. Changes
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly Related PRs
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)
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)
src/openhuman/agent/loop_/instructions.rs (1)
12-18: Scope the “CRITICAL” rule to tool-invocation turns to avoid over-constraining normal replies.The current wording is absolute (“never ... give examples”) while this block also provides an example. Tighten scope so the model understands this restriction applies when emitting tool calls, not to every response.
✏️ Suggested wording tweak
- instructions.push_str( - "CRITICAL: Output actual <tool_call> tags—never describe steps or give examples.\n\n", - ); + instructions.push_str( + "CRITICAL: When you need to call a tool, output actual <tool_call> tags only; do not describe intended tool-call steps in prose.\n\n", + );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/openhuman/agent/loop_/instructions.rs` around lines 12 - 18, The "CRITICAL" rule is currently absolute and over-constrains normal replies; update the string literal that begins "CRITICAL: Output actual <tool_call> tags—never describe steps or give examples." (the text assembled via the instructions.push_str calls) so it explicitly applies only to turns where the model is emitting tool invocations (e.g., say "When emitting <tool_call> tags: ...") and allow examples/descriptions in other contexts; keep the rest of the guidance about providing examples and continuing reasoning (the subsequent instructions.push_str blocks) consistent with this scoped wording change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/openhuman/agent/loop_/parse.rs`:
- Line 379: Replace the generic tracing::warn! call that logs "Malformed
<tool_call> JSON: expected tool-call object in tag body" with a structured
tracing::warn! that attaches the tag text and body length (e.g.,
tracing::warn!(open_tag = %open_tag, body_len = body.len(), "Malformed
<tool_call> JSON: expected tool-call object in tag body")); ensure you reference
the same variables used in this scope (open_tag and body) and keep the original
message text for consistency so production traces include the tag content and
size for diagnostics.
- Line 85: The parse logic lost compatibility with the "<tool>" wrapper causing
tool calls to be dropped; restore "<tool>" to the recognized open-tag list by
adding "<tool>" back into the TOOL_CALL_OPEN_TAGS constant, and ensure the
matching close-tag set (e.g., TOOL_CALL_CLOSE_TAGS or whatever close-tag
constant the parse_tool_calls function uses) includes "</tool>" so
parse_tool_calls correctly recognizes and extracts tool-call blocks (also
run/update any relevant tests that assert telegram provider "<tool>...</tool>"
behavior).
---
Nitpick comments:
In `@src/openhuman/agent/loop_/instructions.rs`:
- Around line 12-18: The "CRITICAL" rule is currently absolute and
over-constrains normal replies; update the string literal that begins "CRITICAL:
Output actual <tool_call> tags—never describe steps or give examples." (the text
assembled via the instructions.push_str calls) so it explicitly applies only to
turns where the model is emitting tool invocations (e.g., say "When emitting
<tool_call> tags: ...") and allow examples/descriptions in other contexts; keep
the rest of the guidance about providing examples and continuing reasoning (the
subsequent instructions.push_str blocks) consistent with this scoped wording
change.
🪄 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: 05ccaede-62ac-48fa-89cd-b6e9069edfc9
📒 Files selected for processing (2)
src/openhuman/agent/loop_/instructions.rssrc/openhuman/agent/loop_/parse.rs
| "[TOOL_CALL]", | ||
| "[tool_call]", | ||
| ]; | ||
| const TOOL_CALL_OPEN_TAGS: [&str; 4] = ["<tool_call>", "<toolcall>", "<tool-call>", "<invoke>"]; |
Contributor
There was a problem hiding this comment.
Restore <tool> tag compatibility to avoid silent missed tool dispatches.
Line 85 removes <tool> from recognized open tags, but src/openhuman/channels/providers/telegram/text.rs still treats <tool>...</tool> as a valid tool-call wrapper. If such output reaches parse_tool_calls, calls can be dropped and downstream fallback in tool_loop.rs/dispatcher.rs may silently no-op.
🔧 Proposed compatibility patch
-const TOOL_CALL_OPEN_TAGS: [&str; 4] = ["<tool_call>", "<toolcall>", "<tool-call>", "<invoke>"];
+const TOOL_CALL_OPEN_TAGS: [&str; 5] =
+ ["<tool_call>", "<toolcall>", "<tool-call>", "<tool>", "<invoke>"];
pub(crate) fn matching_tool_call_close_tag(open_tag: &str) -> Option<&'static str> {
match open_tag {
"<tool_call>" => Some("</tool_call>"),
"<toolcall>" => Some("</toolcall>"),
"<tool-call>" => Some("</tool-call>"),
+ "<tool>" => Some("</tool>"),
"<invoke>" => Some("</invoke>"),
_ => None,
}
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/openhuman/agent/loop_/parse.rs` at line 85, The parse logic lost
compatibility with the "<tool>" wrapper causing tool calls to be dropped;
restore "<tool>" to the recognized open-tag list by adding "<tool>" back into
the TOOL_CALL_OPEN_TAGS constant, and ensure the matching close-tag set (e.g.,
TOOL_CALL_CLOSE_TAGS or whatever close-tag constant the parse_tool_calls
function uses) includes "</tool>" so parse_tool_calls correctly recognizes and
extracts tool-call blocks (also run/update any relevant tests that assert
telegram provider "<tool>...</tool>" behavior).
|
|
||
| if !parsed_any { | ||
| tracing::warn!("Malformed tool_call body: could not parse tag content as JSON or bracket syntax"); | ||
| tracing::warn!("Malformed <tool_call> JSON: expected tool-call object in tag body"); |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Add structured debug context on malformed tag bodies.
Line 379 logs a generic warning only. Include open_tag and body size so parse failures are diagnosable in production traces.
🪵 Proposed logging improvement
- if !parsed_any {
- tracing::warn!("Malformed <tool_call> JSON: expected tool-call object in tag body");
- }
+ if !parsed_any {
+ tracing::warn!(
+ open_tag = %open_tag,
+ body_len = inner.len(),
+ "Malformed tool-call payload: expected tool-call object in tag body"
+ );
+ }As per coding guidelines, src/**/*.rs changes should include substantial debug/trace logging on new or modified flows.
📝 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
| tracing::warn!("Malformed <tool_call> JSON: expected tool-call object in tag body"); | |
| tracing::warn!( | |
| open_tag = %open_tag, | |
| body_len = inner.len(), | |
| "Malformed tool-call payload: expected tool-call object in tag body" | |
| ); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/openhuman/agent/loop_/parse.rs` at line 379, Replace the generic
tracing::warn! call that logs "Malformed <tool_call> JSON: expected tool-call
object in tag body" with a structured tracing::warn! that attaches the tag text
and body length (e.g., tracing::warn!(open_tag = %open_tag, body_len =
body.len(), "Malformed <tool_call> JSON: expected tool-call object in tag
body")); ensure you reference the same variables used in this scope (open_tag
and body) and keep the original message text for consistency so production
traces include the tag content and size for diagnostics.
graycyrus
approved these changes
Apr 1, 2026
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
2 participants
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
instructions.rsthat were merged in fix: reduce agent loop hallucination and improve tool call reliability #156parse.rsthat was merged in fix: reduce agent loop hallucination and improve tool call reliability #156Problem
ReliableProvider.chat(ChatRequest)) but also included prompt changes and a bracket parser that were added during debugginginstructions.rsreplaced the original tool-use instructions with verbose anti-hallucination rules — unnecessary since the model was already calling tools correctly; the issue was the provider not forwarding them[TOOL_CALL]format) was only seen when the agent fell through to the text-onlychat_with_history()path — now that native tools work, the model uses structuredtool_calls, never bracket formatSolution
instructions.rsto its pre-fix: reduce agent loop hallucination and improve tool call reliability #156 state (original tool-use instructions)parse.rsto its pre-fix: reduce agent loop hallucination and improve tool call reliability #156 state (remove bracket parser and[TOOL_CALL]/[/TOOL_CALL]tag support)Submission Checklist
cargo check+cargo fmtpassagentic-v1): shell tool calls execute, file read works, multi-iteration tool loop completes, no Jinja template errorsImpact
Related
Summary by CodeRabbit