Conversation
teknium1
left a comment
There was a problem hiding this comment.
Thanks for carrying the canonical implementation for #60084; the current-main premise is real (agent/tool_guardrails.py:347-352). I found blocking issues before this can safely land.
Problems
agent/tool_guardrails.py:430-431accumulates each hash for the whole turn. Interleaved results such as A/B/A/C/A still warn, although the messages at :438-456 say the last calls were identical.- Native multimodal results reach
_append_guardrail_observationas dictionaries (agent/tool_executor.py:891-896, :1596-1600). A warning from the new path (agent/tool_guardrails.py:448) is then passed toappend_toolguard_guidance(run_agent.py:5728-5729), whose concatenation atagent/tool_guardrails.py:491raises for a dictionary. - The linked issue's production
session_searchempty-success case remains excluded by the< repeated_result_min_charsearly return atagent/tool_guardrails.py:425-427.
Suggested changes
- Track a consecutive semantic-result streak and test interrupted sequences.
- Append guardrail guidance into multimodal text parts/text_summary, then cover both executor paths.
- Add explicit behavior and coverage for short empty-success envelopes from #60084.
Automated hermes-sweeper review.
| return None | ||
|
|
||
| result_hash = _result_hash(text) | ||
| count = self._result_repeat_counts.get(result_hash, 0) + 1 |
There was a problem hiding this comment.
This is a turn-wide histogram, not a repetition streak: A/B/A/C/A reaches the warning threshold even though every duplicate was interrupted by a different result. That conflicts with the later "last {count} tool calls" message and can halt a progressing turn. Track the previous semantic hash plus a consecutive count, resetting when the result changes.
| signature=signature, | ||
| ) | ||
|
|
||
| if self.config.warnings_enabled and count >= self.config.repeated_result_warn_after: |
There was a problem hiding this comment.
For the multimodal case this warning reaches run_agent._append_guardrail_observation, which calls append_toolguard_guidance; that helper concatenates (result or "") + suffix, but the native vision result is a dict. The third repeated image therefore raises TypeError instead of returning this warning. Append to the multimodal envelope's text part/text_summary and add an executor-path regression test.
|
Thanks — all three findings were real. Addressed in 37c27b6a5:
|
…ilure Adds a content-only repetition axis to ToolCallGuardrailController: the loop shape where arguments vary on every call but the result never changes. The existing guards are keyed on tool-call signature (name + canonical args) or on classified failure, so they miss a call that "succeeds" with different arguments each time while returning the same blocked/empty/error body — e.g. execute_code wrapping a fetch against a source that consistently 404s or soft-blocks. The exact-failure counter never fires (failed is false) and the idempotent no-progress tracker is keyed by signature and limited to an allowlist, so neither catches it. A second gap: a repeated vision/multimodal result was never detected, because str(result) embeds a per-call base64 payload that differs even when the meaningful content (a placeholder caption) is identical. One session re-loaded the same image six times with zero guard activity. Repetition is a single consecutive streak (last result hash + counter), not per-turn accumulation, so A/B/A/C/A never fires and the "last N calls" wording in the warn/halt messages is literal. Any different, short, or empty result breaks the streak. - agent/tool_guardrails.py: _track_result_repetition() (content-hash only, independent of tool name, args, and classified failure) and _repetition_text() (keeps multimodal text parts verbatim, reduces each non-text payload to a short digest, so re-loading the same image counts as repetition while distinct images count as progress). Wired into after_call() ahead of the failure/no-progress branches. - hermes_cli/config_defaults.py, cli-config.yaml.example, website/docs/user-guide/configuration.md: repeated_result thresholds under warn_after/hard_stop_after plus repeated_result_min_chars. - run_agent.py: widen the guardrail hand-off annotation to str | dict so multimodal results type-check on the way through. - tests: repeated_result coverage in tests/agent/test_tool_guardrails.py and tests/run_agent/test_tool_call_guardrail_runtime.py, including the varying-args/fixed-result shape, the multimodal blind spot, distinct images as progress, and interleaved/interrupted streaks. Follows the existing axes' design: soft warning by default, hard stop only when hard_stop_enabled is set. Fixes NousResearch#60084 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
b1ae850 to
3628e0f
Compare
|
Was this closed intentionally? #60084 is still open and the guard isn't on main — All three sweeper findings from the 2026-07-15 review were addressed in 37c27b6a5 — consecutive-streak counting so interleaved A/B/A/C/A never fires, multimodal-safe guidance append at both executor call sites, and the short empty-success envelope case from #60084 — and CI was 28/28 green after the 07-31 rebase. It has since gone conflicting against main, which I'm happy to fix. If you'd like it reopened, I'll rebase and push. If it was closed on scope or design grounds instead, a one-line note would help — I'd rather rework it than refile something you don't want. |
What does this PR do?
Adds a content-only repetition guard to
ToolCallGuardrailController: detection of a tool-call loop where the arguments vary on every call but the result never changes.The existing loop guards are keyed on tool-call signature (tool name + canonical args) or on classified failure. That misses a real loop shape observed running a local model in an unattended/gateway session: a tool call that "succeeds" with different arguments each time but keeps returning the same blocked/empty/error-page body — e.g.
execute_codewrapping a web fetch against a source that consistently 404s or soft-blocks a scraper. Neither the exact-failure counter (never fires —failedis false) nor the_no_progressidempotent-result tracker (keyed by signature, and only applied to a fixed idempotent-tool allowlist) catches this.A second, related gap: a repeated vision/multimodal tool result (shape
{"_multimodal": True, "content": [...]}) was never detected as repetition becausestr(result)embeds a base64 image payload that's unique per call even when the meaningful content (a placeholder caption like "Image loaded into your context") is identical — one session re-loaded the same image via a vision tool 6 times with zero guard activity.The guard follows the existing axes' design: soft warning by default, hard stop only when
hard_stop_enabledis set.Related Issue
Fixes #60084
Type of Change
Changes Made
agent/tool_guardrails.py:_track_result_repetition()— a content-hash-only repetition tracker, independent of tool name, arguments, and classified failure. Warns atrepeated_resultwarn threshold (default 3), halts at the hard-stop threshold (default 5, only whenhard_stop_enabled). Results underrepeated_result_min_chars(default 200) are exempt so trivial outputs ("[]","OK") never trip it._repetition_text()— normalizes a tool result before hashing. Multimodal results keep text parts verbatim and reduce each non-text payload (e.g. animage_urldata URI) to a short digest, so re-loading the same image counts as repetition while distinct images (legitimate page-scroll screenshots) count as progress. Plain string results pass through unchanged.after_call()ahead of the existing failure/no-progress branches; a halt short-circuits immediately, a warn is returned if no stronger decision applies.hermes_cli/config.py:repeated_resultthresholds underwarn_after/hard_stop_afterplusrepeated_result_min_charsinDEFAULT_CONFIG["tool_loop_guardrails"].cli-config.yaml.example: document the new keys alongside the existing guardrail axes.website/docs/user-guide/configuration.md: short section for the newrepeated_resultkeys.tests/agent/test_tool_guardrails.py: 5 new tests (see below) plusrepeated_resultcoverage in the existing config-parsing test.How to Test
pytest tests/agent/test_tool_guardrails.py -q— 18 passed. New tests:test_repeated_identical_result_halts_successful_varying_arg_loop(the varying-args/fixed-result loop shape)test_repeated_result_ignores_short_and_distinct_outputs(no false positives on short or genuinely distinct results)test_repeated_multimodal_result_same_image_trips_guard(the vision-result blind spot)test_repeated_multimodal_result_distinct_images_is_progress(distinct images never halt)test_default_config_guardrail_block_matches_dataclass_defaults(DEFAULT_CONFIG stays in sync with the parser defaults)pytest tests/agent/test_turn_context.py tests/run_agent/test_tool_call_guardrail_runtime.py tests/hermes_cli/test_config.py -q— all green (184 passed across the four files combined).tool_loop_guardrails.hard_stop_enabled: true, then have an agent repeatedly fetch a URL that returns the same blocked/error page body with varying query args — the guard warns after 3 identical results and halts after 5.Note: two pre-existing
tests/agentfailures on a cleanupstream/maincheckout (test_anthropic_adapter.py,test_coding_context.py) reproduce identically without this change — they're environment-related, not introduced here.Checklist
Code
fix(scope):,feat(scope):, etc.)pytest tests/ -qand all tests passDocumentation & Housekeeping
docs/, docstrings) — or N/Acli-config.yaml.exampleif I added/changed config keys — or N/ACONTRIBUTING.mdorAGENTS.mdif I changed architecture or workflows — or N/A (no architecture/workflow change)