Skip to content

fix(anthropicapi): report stop_reason "tool_use" for tool-call responses#345

Merged
SantiagoDePolonia merged 3 commits into
mainfrom
fix/messages-stop-reason-tool-use
May 21, 2026
Merged

fix(anthropicapi): report stop_reason "tool_use" for tool-call responses#345
SantiagoDePolonia merged 3 commits into
mainfrom
fix/messages-stop-reason-tool-use

Conversation

@SantiagoDePolonia

@SantiagoDePolonia SantiagoDePolonia commented May 21, 2026

Copy link
Copy Markdown
Contributor

Summary

POST /v1/messages (the Anthropic Messages API ingress endpoint) returned
stop_reason: "end_turn" for responses that contained a tool_use content
block. The Anthropic Messages API contract guarantees stop_reason: "tool_use"
whenever the content holds tool_use blocks — Anthropic SDK tool-use loops
branch on it to decide whether to execute a tool and continue. With
"end_turn", such a client treats the turn as finished and never executes
the tool
, breaking agentic tool-use flows through /v1/messages.

Root cause

OpenAI-family providers report finish_reason: "stop" alongside tool calls
when a tool is forced via tool_choice (confirmed against gpt-4.1-nano).
stopReasonFromFinish only consulted hasToolCalls in the finish_reason == ""
branch, so the "stop" branch won and dropped the tool-use signal.

Fix

stopReasonFromFinish now returns "tool_use" whenever the response carries
tool calls, before the finish_reason switch.

before:  { "stop_reason": "end_turn",  "content": [ { "type": "tool_use", ... } ] }
after:   { "stop_reason": "tool_use",  "content": [ { "type": "tool_use", ... } ] }

User-visible impact

Tool-use loops through /v1/messages now work with OpenAI-family providers
(the common case: a forced tool_choice). Plain text responses are unaffected.

Tests

  • internal/anthropicapi/response_test.go: TestFromChatResponseStopReasons
    gains stop_with_tool_calls and empty_with_tool_calls cases — the
    finish_reason: "stop" + non-empty tool calls combination that the previous
    table never exercised.
  • tests/e2e/release-e2e-scenarios.md: new section 18 with 14 scenarios
    (S96-S109) covering the Anthropic Messages API ingress endpoints —
    non-streaming, cross-provider routing, streaming SSE, polymorphic system,
    multi-turn, count_tokens, forced tool use (the case that surfaced this bug),
    multimodal image, alias resolution, four error-envelope negatives, and
    audit-log visibility.
  • tests/e2e/run-release-e2e.sh: scenario-ID regex widened to accept 3-digit
    IDs so S96+ parse.
  • tests/e2e/release-e2e-findings.md: findings from the validation run.

Validation

  • make test-race and make lint pass (pre-commit).
  • Full release E2E matrix: 109/109 scenarios pass (94/95 existing + 14 new;
    the one existing miss, S91, is a known idle-sleep test-host flake that
    passes on a clean re-run).
  • Verified live with curl against a real gateway — a forced-tool-use
    /v1/messages request returns stop_reason: "tool_use".

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes

    • Corrected stop-reason reporting so tool-invocations are consistently reported as tool_use.
  • Tests

    • Expanded and refined table-driven test coverage for stop-reason and tool-use scenarios; added many new end-to-end scenarios and improved harness parsing for multi-digit IDs.
  • Documentation

    • Updated end-to-end scenario matrix and findings with new scenarios and run results.
  • Chores

    • Streamlined project ignore rules.

Review Change Stack

/v1/messages returned stop_reason "end_turn" for responses that contained
a tool_use block. OpenAI-family providers report finish_reason "stop"
alongside tool calls when a tool is forced via tool_choice, and
stopReasonFromFinish only consulted hasToolCalls in the finish_reason ==
"" branch, so the "stop" branch won and dropped the tool-use signal.

The Anthropic Messages API contract guarantees stop_reason "tool_use"
whenever the content holds tool_use blocks. Without it, Anthropic SDK
tool-use loops treat the turn as finished and never execute the tool.

stopReasonFromFinish now returns "tool_use" whenever the response carries
tool calls, before the finish_reason switch.

Also adds release E2E coverage for the Anthropic Messages API ingress
endpoints (scenarios S96-S109, including the forced-tool-use case that
surfaced this bug) and widens the runner's scenario-ID regex to accept
3-digit IDs.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented May 21, 2026

Copy link
Copy Markdown
Contributor

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: eb7289c3-c77c-419d-84f3-0bfb00f724a3

📥 Commits

Reviewing files that changed from the base of the PR and between 57d4e12 and 8465d2e.

📒 Files selected for processing (2)
  • .gitignore
  • tests/e2e/release-e2e-findings.md

📝 Walkthrough

Walkthrough

This PR fixes a stop-reason contract bug in the Anthropic API response handler, ensuring responses with tool calls always return tool_use regardless of upstream finish values. The fix is validated through expanded unit tests, extended E2E scenario infrastructure, and comprehensive E2E test results showing 107 of 109 scenarios passing.

Changes

Tool-Use Stop-Reason Fix and E2E Validation

Layer / File(s) Summary
Core logic fix and unit tests
internal/anthropicapi/response.go, internal/anthropicapi/response_test.go
stopReasonFromFinish moves the hasToolCalls guard to the beginning to return tool_use before evaluating finish values. The finish == "" branch now returns empty string (tool-use already handled earlier). Unit test table structure expands with name and toolCalls fields; new cases cover tool-call responses with finish_reason: "stop" or empty finish value.
E2E test infrastructure and runner
tests/e2e/release-e2e-scenarios.md, tests/e2e/run-release-e2e.sh, .gitignore
Scenario count increases from 95 to 109. New "Anthropic Messages API ingress" section (S96–S109) documents coverage for /v1/messages and /v1/messages/count_tokens endpoints, including non-streaming responses, provider translation, SSE streaming, polymorphic system input, multi-turn conversation, token counting, tool-use forcing, multimodal images, alias resolution, and negative validation cases. Runner regex loosened to parse multi-digit scenario IDs; .gitignore adjusted for tooling and chart archives.
E2E test results and findings
tests/e2e/release-e2e-findings.md
Test run report (2026-05-21) documents 109 total scenarios with 107 passing. S91 identified as flaky idle-sleep issue (mitigation: caffeinate -i). S102 documented as fixed product bug (stop_reason/tool-calls mismatch on /v1/messages); fix behavior and test coverage details provided. New scenarios S96–S109 confirmed passing with envelope normalization, streaming event sequencing, polymorphic system handling, count_tokens behavior, multimodal input, error envelopes, and audit-log recording validated.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • ENTERPILOT/GoModel#203: Updates to scenario-ID parsing logic in the E2E runner are directly related to this PR's introduction of support for 3-digit scenario identifiers.

Poem

🐰 I hopped through code to mend the rule,
Tool calls now trump a stray finish tool.
Tests and scenarios danced in a line,
One-oh-two fixed, the rest look fine.
E2E bells ring — 107 shine.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately and concisely describes the main fix: correcting stop_reason to 'tool_use' for Anthropic Messages API responses with tool calls. The change to stopReasonFromFinish is the primary driver of this PR.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/messages-stop-reason-tool-use

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@greptile-apps

greptile-apps Bot commented May 21, 2026

Copy link
Copy Markdown

Greptile Summary

This PR fixes Anthropic Messages stop reason handling for tool-call responses. It changes:

  • Reports stop_reason: "tool_use" whenever a rendered Messages response contains tool calls.
  • Adds stop-reason tests for finish_reason: "stop" combined with tool calls.
  • Expands release E2E coverage for /v1/messages and /v1/messages/count_tokens scenarios.
  • Updates the E2E runner to parse three-digit scenario IDs.
  • Broadens local .claude/ ignore coverage.

Confidence Score: 5/5

This looks safe to merge.

  • No blocking issues found in the changed code.

Important Files Changed

Filename Overview
internal/anthropicapi/response.go Updates stop reason mapping so Messages responses with tool-use content report tool_use.
tests/e2e/run-release-e2e.sh Widens scenario heading parsing to support three-digit scenario IDs.
tests/e2e/release-e2e-scenarios.md Adds Anthropic Messages API release scenarios covering routing, streaming, tool use, errors, and audit visibility.

Reviews (2): Last reviewed commit: "chore: ignore the entire .claude directo..." | Re-trigger Greptile

Comment on lines +1838 to +1841
jq -e '.type == "message" and any(.content[]; .type == "text" and (.text | contains("QA_MESSAGES_ALIAS_OK")))' "$RESP_FILE" >/dev/null
curl -fsS -X DELETE "$BASE_URL/admin/aliases" \
-H 'Content-Type: application/json' \
-d "{\"name\":\"$MESSAGES_ALIAS\"}" >/dev/null

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Alias cleanup can skip

S104 creates qa-messages-alias-$QA_SUFFIX, but the cleanup only runs after the request and jq assertion succeed. The runner executes each scenario with set -euo pipefail, so if the /v1/messages call or assertion fails, this DELETE is skipped and the alias is left behind. That breaks the documented self-contained rerun behavior, especially when retrying with the same --qa-suffix or keeping artifacts.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@tests/e2e/release-e2e-findings.md`:
- Around line 91-93: The fenced code block containing `{ "finish_reason":
"stop", "has_tool_calls": 1 }` is missing a language tag (MD040); update that
fence to include a language identifier such as `json` (or `text`) so the block
becomes a proper fenced code block with a language specifier.
- Around line 15-16: The markdown contains inline code spans with trailing
spaces in the regex examples (e.g. `^### S[0-9][0-9] ` and `^### S[0-9][0-9]+
`); remove the trailing space characters inside those backticks so the spans
become `^### S[0-9][0-9]` and `^### S[0-9][0-9]+` respectively to satisfy MD038
(no-space-in-code) while preserving the exact regex text.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: cc8d4796-68ce-47ee-8ab0-57a92cb380cb

📥 Commits

Reviewing files that changed from the base of the PR and between 72a0e68 and 57d4e12.

📒 Files selected for processing (5)
  • internal/anthropicapi/response.go
  • internal/anthropicapi/response_test.go
  • tests/e2e/release-e2e-findings.md
  • tests/e2e/release-e2e-scenarios.md
  • tests/e2e/run-release-e2e.sh

Comment thread tests/e2e/release-e2e-findings.md Outdated
Comment on lines +15 to +16
The runner only parsed 2-digit scenario IDs (`^### S[0-9][0-9] `). It was
widened to `^### S[0-9][0-9]+ ` in `run-release-e2e.sh` so `S96`–`S109` (and

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Remove spaces inside inline code spans in the regex examples.

The inline code spans include trailing spaces, which triggers MD038 (no-space-in-code).

🧰 Tools
🪛 markdownlint-cli2 (0.22.1)

[warning] 15-15: Spaces inside code span elements

(MD038, no-space-in-code)


[warning] 16-16: Spaces inside code span elements

(MD038, no-space-in-code)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/e2e/release-e2e-findings.md` around lines 15 - 16, The markdown
contains inline code spans with trailing spaces in the regex examples (e.g.
`^### S[0-9][0-9] ` and `^### S[0-9][0-9]+ `); remove the trailing space
characters inside those backticks so the spans become `^### S[0-9][0-9]` and
`^### S[0-9][0-9]+` respectively to satisfy MD038 (no-space-in-code) while
preserving the exact regex text.

Comment thread tests/e2e/release-e2e-findings.md Outdated
@codecov-commenter

Copy link
Copy Markdown

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

SantiagoDePolonia and others added 2 commits May 21, 2026 08:33
Address review comments on the findings doc:

- MD040: add the json language tag to the finish_reason evidence fence.
- MD038: render the runner scenario-ID patterns as the slash-delimited
  awk literals (/^### S[0-9][0-9] / and /^### S[0-9][0-9]+ /) so the code
  spans no longer have a trailing space, while keeping the regex text
  exact — the trailing space before the closing slash is significant.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Replace the /.claude/settings.local.json entry with /.claude/ so all
user-specific Claude Code state is ignored, and move it into the existing
"Others" group.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@SantiagoDePolonia SantiagoDePolonia merged commit 05775df into main May 21, 2026
19 checks passed
@SantiagoDePolonia SantiagoDePolonia deleted the fix/messages-stop-reason-tool-use branch June 11, 2026 09:17
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