Skip to content

Rescue a tight tool-call loop sideways instead of killing the turn - #1039

Open
devin-ai-integration[bot] wants to merge 5 commits into
mainfrom
devin/1787687020-loop-sideways-rescue
Open

Rescue a tight tool-call loop sideways instead of killing the turn#1039
devin-ai-integration[bot] wants to merge 5 commits into
mainfrom
devin/1787687020-loop-sideways-rescue

Conversation

@devin-ai-integration

Copy link
Copy Markdown
Contributor

Summary

A tight tool-call loop currently ends the turn: handleToolCallLoopBreak writes a synthetic end_turn and expires the pin. Prod session loom-398a68c6… (2026-08-25 19:31:12) shows why that buys nothing — the pin was cleared, the client resent 37s later, and the HMM re-picked the same arm (gpt-5.6-luna, score 0.88). Net effect: one dead turn on a session that was not actually stuck. Three fixes, all in internal/proxy:

1. Move the session sideways before stopping it. The tight detector now calls handleToolCallLoopSideways first, which reuses the struggle machinery (StruggleEscalationRoster.EscalationTarget: cheapest cluster above the pin's own, else a sideways arm within it) and re-pins with a new translate.ReasonLoopSideways before routing, so this same turn dispatches on a different model instead of dying:

rescue := s.handleToolCallLoopSideways(ctx, sig, count, installationID, sessionKey, loopRole)
if !rescue.Moved {
    loopingModel, loopingProvider := loopAttribution(rescue.LoopingModel, rescue.LoopingProvider, feats.Model, providers.ProviderAnthropic)
    return s.handleToolCallLoopBreak(ctx, ..., loopingModel, loopingProvider, rescue.UserForced, feats.Tokens)
}

The synthetic stop survives as the fallback for every case where a move isn't valid, tracked by an explicit action taxonomy: no_pin, user_forced, already_moved (the pin is already a loop/struggle rescue — looping again there is a task problem, not a misroute), no_target, disabled. The pin write uses context.Background() like the other durable auto-pins; a failed write degrades to the stop. ReasonLoopSideways joins the immutable-sticky set in turnloop.go so the rescued arm can't be re-scored back onto the looping model.

handleToolCallLoopBreak gains a preserveForcedPin argument (same shape as handleNoProgressBreak): the fallback path is now reachable with a /force-model pin in place, and automatic eviction must not clear it.

2. Attribute the loop to the model that was serving it. The detector runs before routing, so feats.Model is the client's inbound baseline — the prod session logged looping_model=claude-fable-5 / anthropic while every one of its 36 turns was served by gpt-5.6-luna / openai. loopAttribution prefers the pin and falls back to the requested model only when there is no pin to read.

3. Exempt poll tools from the identical-args detector. shell_output (and get_output / BashOutput) drain a buffer that changes underneath a byte-identical request — repetition is the tool's normal usage, and it is what tripped the detector in prod. Poll signatures are skipped when counting; they still occupy window positions, so a genuine repeat interleaved with polls still trips.

Not addressed here: struggle escalation needs turns AND wall-clock (30 / 10m), so a fast session like this one (36 turns in 4.5 min) never arms it — that gate is worth revisiting separately.

Link to Devin session: https://app.devin.ai/sessions/e32d7e9712e94512b8ecb03f29c14a37
Requested by: @steventohme

@devin-ai-integration

Copy link
Copy Markdown
Contributor Author

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment, CI, and merge conflict monitoring

@greptile-apps

greptile-apps Bot commented Aug 25, 2026

Copy link
Copy Markdown

PR author is not in the allowed authors list.

@claude

claude Bot commented Aug 25, 2026

Copy link
Copy Markdown

Claude finished @devin-ai-integration[bot]'s task —— View job


Posted an advisory comment-length review with 4 suggestionable nits — won't block merge.

@workweave-bot workweave-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Advisory only — comment-length nits. Won't block merge.

@workweave-bot workweave-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Advisory only — comment-length nits. Won't block merge.

Comment thread internal/proxy/loop_detection.go Outdated
Comment thread internal/proxy/loop_detection.go Outdated
Comment thread internal/proxy/loop_detection.go Outdated
Comment thread internal/translate/force_model.go Outdated
@claude

claude Bot commented Aug 25, 2026

Copy link
Copy Markdown

Claude finished @devin-ai-integration[bot]'s task —— View job


Claude finished @devin-ai-integration[bot]'s task —— View job


Posted an advisory comment-length review with 2 suggestionable nits — won't block merge.

@workweave-bot workweave-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Advisory only — comment-length nits. Won't block merge.

Comment thread internal/proxy/loop_detection.go Outdated
Comment thread internal/proxy/loop_sideways_internal_test.go Outdated
@claude

claude Bot commented Aug 25, 2026

Copy link
Copy Markdown

Claude finished @devin-ai-integration[bot]'s task —— View job


  • Run git diff and analyze added comment blocks
  • Check for 3+ line verbose comment blocks
  • Post suggestion review if needed, or stay silent

View job

Comment thread internal/proxy/loop_detection.go
PinnedUntil: time.Now().Add(pinSessionTTL),
PolicyGroup: targetCluster,
LastServedModel: pin.LastServedModel,
})

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unservable rescue pin skips stop

High Severity

The sideways writer pins Providers[0] after an availableModels check only, with no forcedModelBinding / request-enabled-provider validation. Moved is still true, so the synthetic stop is skipped. runTurnLoop then drops the pin on provider ineligibility and the scorer can re-select the looping model, after which writeNewPin overwrites the rescue.

Additional Locations (1)
Fix in Cursor Fix in Web

Triggered by learned rule: Force-model and user-facing model commands must resolve via catalog, not assume default provider

Reviewed by Cursor Bugbot for commit ef3a5f9. Configure here.

@devin-ai-integration

Copy link
Copy Markdown
Contributor Author

Runtime verification — loop-sideways rescue

Ran the router locally in docker compose from this branch with a mocked Fireworks upstream and a mocked HMM roster sidecar (balanced: [z-ai/glm-5, qwen/qwen3-coder]), a seeded session pin, and hand-crafted tool-call histories over /v1/messages. Every case was also replayed against a docker image built from the merge-base (75710ced) for a real before/after.

Main regression — 5× identical shell_output is no longer stopped

before (main) after (this PR)
body: "→ Tool-call loop detected: shell_output was called 5 times..."
log: Tool-call loop detected; breaking turn tool_name=shell_output
upstream never called
X-Router-Model: z-ai/glm-5
body: MOCK_UPSTREAM_OK...
no loop log lines; upstream request recorded
Sideways re-pin on a real loop (5× identical Bash, pinned session)
X-Router-Decision: loop_sideways
X-Router-Model:    qwen/qwen3-coder
marker:            "→ qwen/qwen3-coder · switched model to break a tool-call loop"
body does NOT contain "Tool-call loop detected"

router.loop_sideways looping_model=z-ai/glm-5 looping_provider=fireworks action=moved
  escalation_target=qwen/qwen3-coder escalation_cluster=balanced policy_group=balanced
  loop_tool=Bash repeat_count=5 window_size=10

mock upstream: + accounts/fireworks/models/qwen3-coder-480b-a35b-instruct
session_pins:  default_mid | qwen/qwen3-coder | fireworks | balanced | loop_sideways | live

The same turn dispatched on the new arm, and the next turn stays pinned (pin_reason=loop_sideways, planner_outcome=skipped) — sticky immutability works.

Fallback paths
  • Replay after rescue → action=already_moved, synthetic stop, looping_model=qwen/qwen3-coder, no upstream call.
  • Fresh session with no pin → action=no_pin, synthetic stop.
  • /force-model session → action=user_forced, synthetic stop, and the user-forced pin is still live afterwards (no eviction).
  • Roster unreachable/invalid → action=no_target, synthetic stop (graceful).
Attribution fix

Same seeded pin (z-ai/glm-5), same payload:

main:     Tool-call loop detected; breaking turn looping_model=claude-sonnet-4-5 looping_provider=anthropic
this PR:  router.loop_sideways               looping_model=z-ai/glm-5        looping_provider=fireworks
Adversarial: poll exemption doesn't mask real loops

shell_output interleaved with 5× identical Bash (window exactly 10) still trips on Bash and moves sideways to qwen/qwen3-coder.

Not exercised at runtime: ROUTER_LOOP_ESCALATION_ENABLED=false, pin-write-failure fallback, and the contents of the loop_escalation_events row.

One prod-relevant nuance found while testing: a genuinely degenerate reply evicts the pin (isDegenerateResponse), so a loop turn on that session afterwards falls back to no_pin rather than moving sideways.

Devin session

@claude

claude Bot commented Aug 25, 2026

Copy link
Copy Markdown

Claude finished @devin-ai-integration[bot]'s task —— View job


  • Run git diff and analyze added comment blocks
  • Check for 3+ line verbose comment blocks
  • Post suggestion review if needed, or stay silent

Posted an advisory comment-length review with 1 suggestionable nit — won't block merge.

@workweave-bot workweave-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Advisory only — comment-length nit. Won't block merge.

Comment thread internal/proxy/loop_detection.go Outdated
@claude

claude Bot commented Aug 25, 2026

Copy link
Copy Markdown

Claude finished @devin-ai-integration[bot]'s task —— View job


  • Run git diff and analyze added comment blocks
  • Check for 3+ line verbose comment blocks
  • Post suggestion review if needed, or stay silent

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

There are 2 total unresolved issues (including 1 from previous review).

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit ad27221. Configure here.

return false
}
return slices.Contains(effortVariantSuffixes, suffix) || translate.IsValidEffort(suffix)
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pro suffix over-excludes distinct models

Medium Severity

sameEngine treats any catalog ID that differs only by a -pro suffix as the same engine, so a tight-loop rescue excludes gpt-5.4-pro and gpt-5.5-pro when the pin is gpt-5.4/gpt-5.5. Those rows are separate models (own prices, no UpstreamID alias), unlike gpt-5.6-luna-pro which maps to the same upstream weights. A looping session can skip a genuine different arm and fall through to a weaker sideways pick or a stop.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit ad27221. Configure here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants