fix(agent): add config opt-out for mixed-batch tool execution permissiveness (#68339) - #71490
Conversation
Related: #68339 reports the policy trade-off motivating this opt-out, while #66317 introduced the permissive mixed-batch behavior. This is not a duplicate, but it needs a maintainer decision on the config policy and a clean rebase: the current three-file diff includes a large full-file line-ending rewrite beyond the focused toggle. |
|
Note from issue author: I truly thank the PR author, the classifier agent, and the maintainers for their work and consideration. It's hard to see problem, and my hope was providing enough evidence and rigor to show it is signal; and maybe AI research hasn't caught up with it yet. |
7959f62 to
498ebd1
Compare
|
Clean rebase done per review request.
All 12 tests in Built on a fresh worktree off |
498ebd1 to
e381e48
Compare
teknium1
left a comment
There was a problem hiding this comment.
Thanks for keeping the default permissive behavior and targeting the existing mixed-batch branch. The current implementation still has the reported mechanism: agent/conversation_loop.py:5752-5756 recognizes a mixed valid/invalid batch and uses the permissive path introduced by 348e9912ff.
Problems
- The proposed public setting is not registered in
hermes_cli/config_defaults.pyor documented inwebsite/docs/user-guide/configuration.md. Consequently,hermes config setwill save it but warn that it is unrecognized (hermes_cli/config.py:4815-4823,4904-4919). - The regression test directly assigns the runtime attribute instead of loading
agent.tool_use_enforcement_permissive_batchesfrom config (tests/agent/test_empty_tool_name_loop_dampening.py:277), so it does not verify the advertised configuration path.
Suggested changes
- Add the default-true key to DEFAULT_CONFIG and document it beside
agent.tool_use_enforcement. - Exercise initialization from an isolated config.yaml in the strict-mode test.
This is an automated hermes-sweeper review.
e381e48 to
bf061a7
Compare
f96079d to
b1ed57f
Compare
|
cc @teknium1 — addressing both inline comments from 2026-07-30 (these were on the previous push; current head rebased onto #1 — register the setting in #2 — config-path regression test. Local: 8 passed in — written by Hermes Agent on behalf of @Enough1122 |
b1ed57f to
53e05c2
Compare
53e05c2 to
db61f57
Compare
|
CI note: the only failing check (Python tests slice 5/12) is test_multiplex_busy_input_mode.py::test_profile_route_and_nonmultiplexed_resolution_preserve_boundaries — a test introduced on main by #83550 (merged 2026-08-11). It fails identically on unrelated PRs in the same window (e.g. #82222, #83834), and the implementation it tests (gateway/run.py) is untouched by this PR. The failure is on main, not in this branch; a rebase onto a main that contains the fix (or a maintainer fixing #83550's test) will unblock. |
7660b97 to
b330d1f
Compare
|
Both review points addressed (branch rebased onto current main):
Also dropped the large full-file line-ending rewrite from the earlier diff — the branch now carries only the focused toggle + tests + docs (5 files, +162). |
|
Thanks for the note — the evidence and rigor in #68339 were exactly what made the opt-out worth building, and the strict-mode toggle (defaulting to the current permissive behavior) is designed so the negative-reinforcement constraint stays available to anyone who wants it. |
b330d1f to
8e695d1
Compare
|
Rebased onto a clean main point (16b54e2) to clear the CI failure. The failing slice 4/12 test ( |
8e695d1 to
045f061
Compare
bcc73ce to
ca47ee3
Compare
ca47ee3 to
3e9859b
Compare
b5784c3 to
8e6ebf6
Compare
|
Rebased onto current
Force-pushed to _(Posted on behalf of @Enough1122 via Hermes Agent) |
…iveness (NousResearch#68339) Clean rebase of NousResearch#71490 per reviewer request - line-ending noise removed, diff now contains only the focused toggle. - agent/agent_init.py: read agent.tool_use_enforcement_permissive_batches (default True, try/except fallback to preserve existing-install safety) - agent/conversation_loop.py: gate _mixed_invalid_batch on the toggle so strict mode voids the whole turn (pre-#348e9912f behavior) - tests/agent/test_empty_tool_name_loop_dampening.py: add regression test test_mixed_batch_strict_mode_voids_batch_when_permissive_disabled Closes NousResearch#68339
8e6ebf6 to
0f5cfac
Compare
|
Follow-up: rebased once more onto latest _(Update on behalf of @Enough1122) |
|
Closing as part of an open-queue triage — consolidating my open work so review attention goes to the highest-value changes. Nothing here is abandoned: happy to reopen (or rebase fresh) if there's interest or reviewer bandwidth. |
Summary
Adds a per-install config opt-out for the mixed-batch tool-execution behavior introduced in #348e9912f (via #66317). Default behavior is unchanged; installs that observe the deepseek/qwen over-emission described in #68339 can set
agent.tool_use_enforcement_permissive_batches: falseto restore the pre-#348e9912f "void the batch on any invalid name" path.What this PR does
agent.tool_use_enforcement_permissive_batchesfromconfig.yamlduring agent init. Defaulttrue(= current behavior, no change for existing installs).agent/conversation_loop.py, gates the_mixed_invalid_batchbranch on the toggle. Whenfalse, the existing all-invalid-batch path runs instead, which strikes the whole turn out and emits the "Skipped: another tool call in this turn used an invalid name" message for every valid sibling. That "Skipped" message is the negative-reinforcement signal that the issue reports is missing for enforcement-gated models.Why opt-out instead of flipping the default
The default in #348e9912f was set for a real reason: gpt-5.6-class models at 350K+ input emit batches like "6 valid calls + 1 blank-name rider" and pre-#348e9912f would void all 6 valid calls. Flipping the default back to strict would re-break those sessions. The opt-out lets each install pick the side that matches its model mix and workload.
Tests
Added
test_mixed_batch_strict_mode_voids_batch_when_permissive_disabledintests/agent/test_empty_tool_name_loop_dampening.py. It sets the toggle tofalse, sends a[("todo", "{}"), ("", "{}")]batch, and asserts every valid sibling in that batch gets the "Skipped" message (the strict path), and the blank-name call still gets its own terse anti-priming error (the anti-priming contract is independent of the batch-level toggle). Resets the toggle in afinallyblock so the rest of the file's tests get the default permissive behavior.All 12 tests in
test_empty_tool_name_loop_dampening.pypass:Behavior contract
agent.tool_use_enforcement_permissive_batches: true(default): mixed batches execute valid + error the invalid. This is current behavior, unchanged by this PR.agent.tool_use_enforcement_permissive_batches: false: mixed batches strike out the whole turn, valid siblings get the "Skipped" message. The 3-strike_invalid_tool_retriescounter still advances on a fully-invalid turn, so the strike-out path still works.Diff scope
3 files, +75/-1:
agent/agent_init.py: +22 (config read, default-true, with try/except fallback to preserve existing-install safety)agent/conversation_loop.py: +14/-1 (gate the existing branch on the toggle)tests/agent/test_empty_tool_name_loop_dampening.py: +40 (one new test)Closes #68339