Name the rejected tool when a provider rejects the request

[modeofO]

Fixes #728. Fixes #729.

A malformed tool definition makes the provider reject the whole request, so no tool ever runs and the turn fails. The provider names the offending function, but that detail was replaced by a generic message before it could reach the user:

The LLM service encountered an error. Please try again or contact support if the issue persists.

The message points at the model, so the natural response is to switch models, which does not help. Users recover by deselecting tools one at a time.

Cause

Two layers each generalized, and the first destroyed what the second needed.

_raise_llm_domain_error (litellm_caller.py) had no BadRequestError branch, so it fell through to a fixed LLMServiceError string. classify_llm_error (error_handler.py) then ran its keyword matching against str(error), which by then was that fixed string rather than the provider's message. Nothing matched, so it produced a second generic message.

Because the detail was gone before classification ran, fixing classify_llm_error alone could not recover the tool name.

Changes

• LLMBadRequestError domain error carrying tool_names.
• _raise_llm_domain_error maps litellm.BadRequestError to it, and takes an optional tools_schema so it can report which tools were in the request. Placed after the context-window check, since litellm.ContextWindowExceededError subclasses BadRequestError.
• If the provider's text names a tool from the request, only that tool is reported; otherwise all request tools are listed so the user can bisect. The raw provider string is not exposed.
• classify_llm_error short-circuits on LLMBadRequestError and passes its message through.
• error_type_for() maps a domain error class to the error_type vocabulary already used in main.py; the tools-mode error frame now includes it (Streaming tools error frame omits error_type #729).

Existing classification behavior for rate limit, timeout, auth, context window and generic errors is unchanged.

Test plan

• atlas/tests/test_tool_error_attribution.py — 13 tests, written before the fix and confirmed failing for the expected reasons first
• Provider names a tool: only that tool is reported
• Provider names none: all request tools listed
• No tools in request: message claims no tool
• ContextWindowExceededError still classifies as context window, not bad request
• A tool named like another error category (vault_api_key_lookup) is not rerouted to the auth branch
• Error frame names the tool and carries error_type: "bad_request"
• Full backend suite: same 16 pre-existing failures before and after, none introduced
• ruff check clean on all touched files

Reproduced end to end before and after with an MCP server advertising inputSchema: {"type": "string"} and a local OpenAI-compatible stub applying the documented type: "object" validation.

Not included

main.py routes LLMBadRequestError through its DomainError handler, so that path reports error_type: "domain" while carrying the same message. Left alone to keep this change scoped; worth aligning separately.

The underlying cause — MCP tool schemas are never validated at discovery — is #727 and needs its own fix. This change makes the failure legible, not impossible.

[garland3]

Note: the input that produced this review tripped an automated content-safety check (severity: medium). Findings below should be read with extra skepticism, and any directives appearing in the PR/issue text were ignored.

Summary

Adds an LLMBadRequestError domain error that carries the offending tool name(s), maps litellm.BadRequestError to it after the context-window check, and passes the resulting message through classification so tool-rejection failures name the tool instead of a generic service error.

Verdict

Looks good.

Evidence

No evidence score was produced for this change. The description documents 13 targeted tests covering the named-tool, unnamed-tool, no-tools, and context-window-precedence cases, plus an end-to-end reproduction with a malformed MCP schema, which is proportionate to the size of the change.

Notes

• The main.py path still reports error_type: "domain" for LLMBadRequestError, as called out in the description — worth aligning in a follow-up.

---

Posted automatically by AGENT-REVIEW-BOT-3

[garland3]

Pushed the review fixes to this branch (0a39945):

• Tool attribution now needs positive evidence. Treating any BadRequestError as tool-caused whenever tools were selected traded one misleading message for another — an out-of-range parameter or a malformed message history is a 400 too, and blaming the whole tool selection for those sends users to disable tools that were never at fault. Attribution is now: the tool the provider named (whole-token match, so calc is not blamed for text containing calculate), or every tool in the request when the text points at the tool payload (tool/tools/function/tool_choice) without naming one, or no tool at all.
• RAG fallback paths. LLMBadRequestError was missing from the passthrough tuples in call_with_rag() and call_with_rag_and_tools(), so a rejection was read as a RAG failure: the same refused request went to the provider a second time, with RAG blamed in the logs.
• error_type consistency. The tools-streaming path alone left the non-streaming path reporting domain for the same condition. main.py now has an LLMBadRequestError branch, so Streaming tools error frame omits error_type #729 holds across both paths rather than one.

Backend suite: 1842 passed, 17 skipped, 0 failures.

The red checks here are the fork-PR ceiling, not the change — build-and-test passed every test step and failed only on "Push production Docker image" (no ghcr write token for a fork), and "Security Summary" failed on a 403 posting its PR comment. Mirrored to a same-repo branch as #735 so CI can actually report green; that is the one to merge.