Skip to content

feat(combo): add agentic-stall response classifier - #13954

Open
patrykkopycinski wants to merge 5 commits into
cheaper-inference:release/v3.8.53from
patrykkopycinski:feat/agentic-stall-classifier
Open

patrykkopycinski wants to merge 5 commits into
cheaper-inference:release/v3.8.53from
patrykkopycinski:feat/agentic-stall-classifier

Conversation

@patrykkopycinski

Copy link
Copy Markdown
Contributor

Summary

Adds a standalone agentic-stall response classifier for the combo failover path.

Some upstream providers answer a mid-agentic-turn request — tools present, conversation tail a tool result — with finish_reason: stop and a progress report as content, and zero tool calls. Agent harnesses read stop + no tool call as turn-over, so the client re-prompts, the model reports progress again, and the loop never converges. Every existing quality gate passes the response: it is a valid, non-empty completion.

This PR lands the classification layer only: pure functions that decide whether a 200 response is that stall shape. It adds no imports, no call sites, and changes no existing behaviour.

Related Issues

I did not claim Closes #13691: this PR classifies the stall but does not yet act on it. The classification has to land before a retry/failover caller can consume it. See Reviewer Notes.

Validation

  • Change type: routing
  • Focused tests and category gates from the golden path — bash scripts/quality/test-scoped.sh → 66/66 pass
  • npm run lint — cannot run in my environment: ESLint 10.9.1 crashes loading react/display-name (contextOrFilename.getFilename is not a function) on any file, including pristine origin/main files. Repo/environment debt, not this diff. prettier --check passes.
  • Reconciled with the current active release base; focused checks rerun afterward — branched from origin/main @ a3ca33fa6, which was still the tip after fetch
  • Production-code changes include a new or updated automated test in this PR

Classifier

Condition Requirement
Request shape tools present and conversation tail is a tool result (OpenAI role: "tool"; Anthropic last user message carries tool_result)
Response shape finished with stop / end_turn and contains no tool calls
Content shape summary/narration block (<summary>…, # Summary, Summary: …) or short structureless prose reporting progress

Three terminal protocols are exempt because their response is turn-over by design:

  1. a genuine blocked-state report (Blocked: … still OPEN, Cannot proceed …),
  2. an intentional-silence control token ([SILENT], NO_REPLY, and the CJK equivalents) — an autonomous lane's "nothing to report" reply,
  3. a literal the request itself quoted as the required terminal reply — a lane briefed with "if nothing is worth saving, just say 'Nothing to save.' and stop" is answered exactly that way by design; failing over re-runs the same deterministic turn.

Exemption 3 is deliberately narrow: verbatim, case-insensitive, whole-response match against a literal present in the request, under a length cap. A model drifting into a summary cannot satisfy it, because summarization text is not quoted in the brief. If the brief contains no literal, the function returns false and the normal path is unchanged.

Tests Added Or Updated

  • tests/unit/agentic-stall.test.ts — new, 66 tests

Covers: the request-shape gate in both API shapes, the narration heuristic, full classification across OpenAI/Anthropic × JSON/SSE, the positive controls (no tools pending / a real tool call / structured output must not classify), the kill switch, the clone-only contract on the response body, and the three exemption classes including their negative controls.

Coverage Notes

  • open-sse/services/combo/agenticStall.ts is covered by the new unit suite; there is no uncovered branch reachable from the exported API.
  • Mutation-tested (the suite must fail when the logic breaks, not merely pass):
Mutation Result
silence exemption forced always-true killed (9 failures)
instructed exemption forced always-true killed (9 failures)
instructed exemption forced never-true killed (6 failures)
length cap removed at all three enforcement layers killed (2 failures)

The cap is enforced at three points (the extraction filter, the whole-response check, and the extraction regex). Disabling any single layer is an equivalent mutant — the others still enforce it — which is intentional defence in depth; removing all three is what the tests catch.

Reviewer Notes

  • This PR deliberately adds no call sites. open-sse/services/combo/combo.ts is where the failover decision belongs, and that file has diverged substantially on active integration branches. Landing the classifier first keeps this change reviewable in isolation and avoids a large conflict-prone diff. A follow-up PR can wire it into the combo loop once a call site is agreed.
  • The failover must not retry blindly. fix(providers): devin-cli-agentic: SWE-2 models return <summary> envelopes as final content (finish_reason=stop), breaking agent tool loops #13691 reports that re-prompting SWE-2 for a proper final answer frequently returns <summary> again. Failover to the next combo member is the remedy shape; retrying the same member is not.
  • One divergence worth calling out for reviewers: exemption 2 mirrors the marker set an agent harness already treats as silence, so the two cannot drift on what counts as silence. Those markers are listed in SILENCE_MARKERS — if that list should be narrower or wider for OmniRoute, say so and I will adjust.

Adds a standalone, side-effect-free classifier that detects when a model
answers a tool-result turn with prose instead of the tool call the loop is
waiting for — the "agentic stall" that leaves an agent hanging until timeout.

This commit is the classifier and its tests ONLY. It is not wired into any
dispatch path here: the wiring in services/combo.ts depends on refactors that
do not exist on this branch, so porting it would mean rewriting unrelated code.
Landing the pure logic first keeps this reviewable and lets wiring follow in a
separate PR.

Design notes:
  - Pure functions, zero imports; never mutates or consumes the caller's
    Response (clone-only contract, covered by a test).
  - Returns null on anything it cannot confidently parse, so an unknown
    content-type can never break the request path.
  - Kill switch: OMNIROUTE_AGENTIC_STALL_FAILOVER=0 disables classification.

Two exemptions prevent false positives observed in production:
  - Intentional silence: an exact "[SILENT]" (or blocked-state) reply is a
    valid answer, not a stall.
  - Instructed terminal echo: when the request itself instructs the model to
    reply with a specific short literal, echoing it verbatim is compliance.
    Matching is verbatim and length-capped, so a drifted paraphrase still
    classifies as a stall.

Tests: 64 cases over both providers x JSON/SSE, both exemptions, the kill
switch, the null-safety path and the no-consume contract.
The previous run failed at `npm run check:node-runtime` (ERR_MODULE_NOT_FOUND:
tsx) even though the install step reported success, and every downstream job
was skipped as a result. No other open PR fails that step. Re-running to
distinguish a transient install/cache failure from a real one.
@patrykkopycinski

Copy link
Copy Markdown
Contributor Author

CI attribution — all 22 failures are pre-existing on the PR base, not this diff.

This PR touches 2 files (open-sse/services/combo/agenticStall.ts, tests/unit/agentic-stall.test.ts). It does not touch package.json, package-lock.json, or any workflow.

Root cause of the 15 check:node-runtime failures (single cascade):

Error [ERR_MODULE_NOT_FOUND]: Cannot find package 'tsx'

node_modules is never populated, so node --import tsx scripts/check/check-supported-node-runtime.ts cannot start. Every downstream job is then skipped.

Why node_modules is empty — the install step is a false green. ./.github/actions/npm-ci-retry runs npm ci 3 times, all 3 fail, and the step still reports success:

npm error `npm ci` can only install packages when your package.json and
npm error package-lock.json or npm-shrinkwrap.json are in sync.
npm error Please update your lock file with `npm install` before continuing.
...
npm ci attempt 3/3 after transient failure
##[end-action id=__self.__run;outcome=success;conclusion=success;duration_ms=72497]

The retry loop's last line is exit "$exit_code" on attempt 3, so a genuine sync error is retried twice (20s + 40s) and then reported as a pass. The step's own conclusion=success is what hides it.

Attribution — reproduced on the untouched base commit:

Check Result
PR base a3ca33fa6 (= current main tip)
This PR touches lockfile/manifest/workflows no — 2 source/test files only
npx npm@11 ci --dry-run --ignore-scripts on pristine a3ca33fa6 RC=1 — "can only install packages when your package.json and package-lock.json … are in sync"
Same command with local npm 10.9.8 RC=0

The failure is npm-major-specific (Node 24 ships npm 11; this machine has npm 10.9.8). Drift is in packages/browser-pool: manifest requires playwright: 1.62.1, and npm 11 rejects the tree while npm 10 accepts it.

Already being fixed upstream: #13865 (fix(deps): restore npm 11 installs on main) targets exactly this. This PR should go green once that lands; no duplicate lockfile change here.

Other failures, also not this diff: Quality Gates (Extended) (osv-scanner vulnerability ratchet), Docs Sync (Strict) (check:docs-all), and Vitest/Bun SQLite/Quality Ratchet — all in the same dependency-bootstrap blast radius or already red on base.

What this PR does verify locally, on the same base commit: the classifier suite passes through the repo's own gate, bash scripts/quality/test-scoped.sh → 66 pass / 0 fail, plus prettier --check clean. open-sse typecheck reports 2 errors on pristine a3ca33fa6 as well (pre-existing, baseline not widened).

@diegosouzapw diegosouzapw added the deferred-v3.8.52 Grande demais / suspeito para o lote atual; precisa de sessão dedicada no ciclo v3.8.52 label Sep 25, 2026
@diegosouzapw diegosouzapw changed the title feat(combo): add agentic-stall response classifier [defer] feat(combo): add agentic-stall response classifier Sep 25, 2026
@diegosouzapw diegosouzapw removed the deferred-v3.8.52 Grande demais / suspeito para o lote atual; precisa de sessão dedicada no ciclo v3.8.52 label Oct 1, 2026
@diegosouzapw diegosouzapw changed the title [defer] feat(combo): add agentic-stall response classifier feat(combo): add agentic-stall response classifier Oct 1, 2026
@diegosouzapw
diegosouzapw changed the base branch from main to release/v3.8.52 October 8, 2026 01:09
@diegosouzapw

Copy link
Copy Markdown
Collaborator

Hi @patrykkopycinski — closing this pull request to clear the open queue. The branch feat/agentic-stall-classifier is kept, so the commits stay. Comment here if it should be reopened.

@diegosouzapw

Copy link
Copy Markdown
Collaborator

Hi @patrykkopycinski — reopening this pull request. The close was a mistake, and it is back in the merge queue. The branch feat/agentic-stall-classifier was kept.

@diegosouzapw diegosouzapw reopened this Oct 10, 2026
@diegosouzapw diegosouzapw added the deferred-v3.8.53 Feat PRs held for v3.8.53. Keep open. Do not merge into v3.8.52. label Oct 13, 2026
@diegosouzapw diegosouzapw changed the title feat(combo): add agentic-stall response classifier [defer] feat(combo): add agentic-stall response classifier Oct 13, 2026
@diegosouzapw
diegosouzapw changed the base branch from release/v3.8.52 to release/v3.8.53 October 19, 2026 13:36
@diegosouzapw

Copy link
Copy Markdown
Collaborator

Re-homed to release/v3.8.53: v3.8.52 entered its release freeze, so the branch now belongs to the release captain and development continues on the next cycle. Nothing is wrong with this PR — it just needed a live base. No action needed from you; CI will re-run against the new base.

@diegosouzapw diegosouzapw removed the deferred-v3.8.53 Feat PRs held for v3.8.53. Keep open. Do not merge into v3.8.52. label Oct 20, 2026
@diegosouzapw diegosouzapw changed the title [defer] feat(combo): add agentic-stall response classifier feat(combo): add agentic-stall response classifier Oct 20, 2026
diegosouzapw and others added 3 commits October 22, 2026 16:24
…s env var

OMNIROUTE_AGENTIC_STALL_FAILOVER now defaults OFF (enabled only by 1/true),
matching the project rule that new behavior-changing flags are opt-in. The
variable is documented in .env.example and docs/reference/ENVIRONMENT.md so
check:env-doc-sync passes, and a changelog fragment is added.

Tests: the classify cases enable the flag explicitly; new cases prove the
default-OFF and 0/false/empty/1/true behavior (red before the change).
Resolves the .env.example and docs/reference/ENVIRONMENT.md conflicts by keeping both sides: the OMNIROUTE_AGENTIC_STALL_FAILOVER entry plus every variable the release added.

Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>
…omplexity ratchet

quotedTerminalLiterals, extractFromSse and classifyAgenticStallResponse exceeded the complexity/cognitive-complexity threshold (15) of eslint.complexity-ratchets.config.mjs. Extract message-part text and quoted-literal collection, per-line SSE parsing with OpenAI/Anthropic folding helpers (Anthropic events via a per-type handler map), and response-signal reading plus the stop/no-tool-call checks. Behaviour is unchanged; the existing 68-test suite covers the JSON and SSE paths.

Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>

This branch has not been deployed

No deployments
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