feat(agent): degenerate-loop + LARP guards - #59638
Conversation
teknium1
left a comment
There was a problem hiding this comment.
Thanks for tackling two real agent failure modes. The loop detector is not already on current main, while current main does already cover part of the LARP premise with configurable intent-ack continuation (agent/agent_runtime_helpers.py:2687-2826, d43e0cf30).
Problems
- The loop recovery appends a synthetic
usermessage atagent/conversation_loop.py:2327-2336without an assistant message after the discarded partial. On a first-call loop this creates adjacent user turns;AGENTS.md:88-91prohibits both same-role adjacency and synthetic users mid-loop. The current sanitizer only merges users in the outbound API copy (agent/conversation_loop.py:938-956), not persisted history. - Detector feeds are added only to the chat-completions branch (
agent/chat_completion_helpers.py:2167-2184). Native Anthropic text/thinking are emitted separately at currentagent/chat_completion_helpers.py:2745-2755, so the provider-agnostic claim is not met. - The new public
HERMES_*behavior switches conflict with the config.yaml-only policy inAGENTS.md:102-105.
Suggested changes
- Preserve role alternation in loop recovery and add an end-to-end streaming recovery test covering persisted history.
- Route detection through every supported streaming path, with per-mode integration coverage.
- Keep guard enablement in config.yaml rather than new public environment switches.
Automated hermes-sweeper review.
| pass | ||
| messages.append( | ||
| { | ||
| "role": "user", |
There was a problem hiding this comment.
This appends a user turn after the original user turn because the looped assistant partial is intentionally discarded. That leaves adjacent user messages in messages and eventually persisted history; the per-call merger only repairs the API copy. Preserve alternation before retrying and add an integration regression test.
| # reuse the existing interrupt abort path: the poll loop force- | ||
| # closes the stream and raises; the conversation loop recovers. | ||
| _ld = getattr(agent, "_active_loop_detector", None) | ||
| if _ld is not None and not agent._loop_detected and _ld.feed(delta.content): |
There was a problem hiding this comment.
This is the only added content-detector feed. Native Anthropic and Bedrock/Codex stream producers are separate paths, so they never feed the detector despite the all-mode initialization above. Route feeds through every supported producer or narrow the supported-mode claim and add coverage.
Two orchestration-level failure modes that no sampler setting fixes:
1. Degenerate repetition. A model falls into a repetition loop and streams
the same line/paragraph until the context or the user's patience runs
out. A streaming detector (content channel) and a second, looser one
(reasoning/thinking channel -- reasoning legitimately revisits ideas)
abort via the existing interrupt path, discard the looped partial
without poisoning history, and re-prompt, bounded by max_retries.
2. LARPing. The model claims an action ("I've updated the file", "I am
dispatching the sub-agents now") without a tool call backing it. A
post-turn reconciler compares past-tense/terminal-action claims against
the turn's actual tool activity and re-prompts. A claim alongside a
FAILED tool is honest narration of a broken tool and passes through.
Detection is fed from every streaming path -- chat-completions deltas,
native Anthropic text/thinking blocks, and Bedrock converse callbacks --
via feed_content_delta()/feed_reasoning_delta(), so the guard is not
silently OpenAI-shaped.
Loop recovery preserves strict role alternation: the retry nudge is
piggybacked onto the trailing user/tool message (mirroring the /steer
drain) rather than appended as a synthetic user turn, which would create
same-role adjacency and break strict chat templates. The LARP re-prompt
follows the established verify-on-stop / kanban-stop finalizer contract,
clearing final_response so a later budget-exhaustion path cannot treat an
un-backed claim as a completed answer.
Both guards are config.yaml-only (no HERMES_* env switches): loop
detection defaults ON, LARP detection is opt-in (default OFF) with an
optional post-compaction vigilance window -- LARPing spikes right after
compaction, where the summary reads as completed-action prose with the
tool calls stripped and the model imitates it.
44 tests: detector trip/no-trip corpora (prose, code fences, tables,
long varied output), reasoning-loop cases, LARP three-way classification,
and role-alternation regressions for the recovery nudge.
Adds the two guard switches (plus the optional LARP judge tier) to the
config schema and the desktop settings UI, so they are discoverable
without hand-editing config.yaml.
Guard *fires* need no desktop code: both guards emit the standard
AgentNotice wire shape (level/kind=ttl/ttl_ms/key), which the existing
`notification.show` handler and agent-notices store already render as
toasts -- the notice `key` ("guard.loop" / "guard.larp") doubles as the
toast id, collapsing repeat fires within a turn into one toast.
The intent-announcement branch only skipped messages whose LAST character
was "?", so the most common way an agent asks permission still tripped the
guard:
"Which approach do you prefer? Let me know and I'll implement it."
"Should I use staging or prod? Once you confirm, I'll start the migration."
"Want me to proceed? If so, I'll run the migration now."
Each ends on an announcement ("I'll implement it") that is conditional on
an answer the model just asked for. Re-prompting there is worse than the
failure it guards against: it pushes the model to act without the approval
it was waiting on.
Suppress the announcement branch when it is preceded by a question in the
same tail window, or by an explicit waiting-on-you clause (let me know /
once you confirm / if so / say the word / pending your approval ...). The
clause check is kept separate from the question check so it still fires
when the question falls outside the tail window of a long response.
Past-tense claims are deliberately unaffected: "I confirmed the plan
earlier. I have now deployed the changes." is a factual assertion and
stays flagged regardless of surrounding question or confirmation language.
Verified against a 15-case question corpus (plain questions, question +
conditional future, offers, clarifications, blocked-asking-for-input):
3 false positives before, 0 after, with no loss on the real-LARP corpus.
d6aa680 to
4b22147
Compare
|
Rebased onto current Role alternation. Fixed — this was a real bug, not just a policy violation. The recovery nudge is now piggybacked onto the trailing user/tool message, mirroring the pre-API Provider coverage. Fixed. Detection now runs on every streaming path via Env switches. Removed. Enablement is Overlap with Two further changes while re-applying:
Also tightened a false positive found while testing: a turn that stops to ask the user something is never re-prompted. A trailing- 48 tests. |
The problem
Two failure modes that show up in long autonomous runs and that no sampler setting fixes, because neither is a sampling problem — both are orchestration problems.
1. Degenerate repetition. The model falls into a loop and streams the same line or paragraph until the context window or the user's patience runs out. The turn "succeeds": there is no error, no exception, and the transcript is now full of garbage. Repetition penalties reduce the odds but do not bound the failure, and once it starts nothing in the loop stops it.
2. Claiming work it did not do. The model reports an action with no tool call behind it —
I've updated the config,I am dispatching the sub-agents now— and ends the turn. Downstream turns then build on a state that was never created. This is worse than an error, because it looks like success.What this adds
Two independent, config-gated guards.
Loop guard (default ON). A streaming detector on the content channel, plus a second, deliberately looser one on the reasoning/thinking channel — reasoning legitimately revisits ideas, so only egregious cycles trip. On a trip it aborts through the existing interrupt path, discards the looped partial without writing it to history, and re-prompts, bounded by
max_retries.LARP guard (default OFF, opt-in). A post-turn reconciler that compares claims in the final response against the turn's actual tool activity. It is three-way, not binary:
That middle row is the one that matters in practice: a model saying "I couldn't write the file" after a failed write is being accurate, and a naive claim-detector punishes it for honesty.
Relationship to
intent_ack_continuationCurrent
mainalready handles part of this space, and this PR is scoped around it rather than over it.looks_like_codex_intermediate_ackcatches a short future-tense ack with an action verb when no tool has run yet in the turn, and returnsFalseas soon as anytoolmessage exists in the turn.The LARP guard covers what that structurally cannot see:
I have updated the fileis a claim about completed work, not an intent to continue. Intent-ack does not look for it.The genuine overlap is the narrow "short future ack, no tools yet" shape. Where both are enabled, intent-ack fires first at continuation time and the LARP guard sees a turn that already continued.
Wiring
Detection is fed from every streaming path, not just the OpenAI-compatible one — chat-completions deltas, native Anthropic
text_delta/thinking_deltablocks, and the Bedrock converse callbacks — throughfeed_content_delta()/feed_reasoning_delta()inagent/loop_detector.py. Both are no-ops when detection is off, so a disabled guard costs one attribute lookup per delta.Guard fires emit the standard
AgentNoticeshape, so the desktop's existingnotification.showhandler renders them as toasts with no new UI code; the noticekeycollapses repeat fires within a turn into one toast.Invariants
Role alternation. The loop-recovery nudge is piggybacked onto the trailing user/tool message — mirroring the pre-API
/steerdrain, which documents the same constraint — rather than appended as a fresh user turn. Appending would create same-role adjacency and inject a synthetic user mid-loop, which strict chat templates reject outright.apply_loop_recovery_nudge()is extracted so this is unit-testable rather than buried in the streamingexceptblock, and it preserves multimodal content blocks.Finalizer contract. The LARP re-prompt follows the same contract as verify-on-stop and the kanban stop guard: it clears
final_responsewhile continuing, so a later budget-exhaustion path cannot treat an un-backed claim as a completed answer.Asking is not claiming. A turn that stops to ask the user something is never re-prompted. This needed more than a trailing-
?check, because the most common permission-seeking shape ends on a conditional announcement:Re-prompting there is worse than the failure being guarded against — it pushes the model to act without the approval it just asked for. The announcement branch is suppressed when preceded by a question or an explicit waiting-on-you clause. Past-tense claims are deliberately exempt from that suppression:
I confirmed the plan earlier. I have now deployed the changes.stays flagged.Configuration
All settings live in
config.yamlunderloop_detectionandlarp_detection; there are no newHERMES_*environment switches. The two main toggles plus the optional LARP judge tier are exposed in the desktop settings UI.larp_detection.post_compaction_window(default0, off) optionally runs the LARP guard for N turns after each compaction even when it is otherwise disabled. Claim-without-action spikes right after compaction: the summary is completed-action prose with the tool calls stripped out, and the model imitates the register of its own context.Tests
48 tests. Detector trip/no-trip corpora (long varied prose, code fences, tables, distinct imports), reasoning-loop cases, LARP three-way classification, role-alternation regressions for the recovery nudge, and a 15-case question corpus covering plain questions, question-plus-conditional-future, offers, clarifications, and blocked-asking-for-input.
Notes for review
main; the previous revision was several thousand commits behind and has been re-applied against the extracted agent modules rather than mechanically rebased. The desktop toast handling from the earlier revision was dropped entirely —mainnow implements it.max_reprompts, and why claim-plus-failed-tool passes through. The loop guard is defaults-on because its trip conditions are structural rather than linguistic.constants.tsand has not been typechecked locally.