Skip to content

fix(sagemaker): report max_tokens for a non-streamed tool call cut off by the token limit - #5619

Open
chrikrah wants to merge 1 commit into
strands-agents:mainfrom
chrikrah:fix/sagemaker-length-tool-call
Open

chrikrah wants to merge 1 commit into
strands-agents:mainfrom
chrikrah:fix/sagemaker-length-tool-call

Conversation

@chrikrah

Copy link
Copy Markdown
Contributor

Human Overview

Truncated SageMaker tool calls stop as max_tokens.

Description

Unstreamed, sagemaker.py:596 replaces inherited max_tokens for finish_reason: length with tool_calls, so the tool runs on truncated arguments, bypassing recover_message_on_max_tokens_reached.

# stub endpoint, stream False: get_weather(location="Pa") with finish_reason "length", then text
$ python repro_sagemaker.py   # base 15ca21f15
no exception
tool ran: ['Pa'] | invoke_endpoint calls: 2
$ python repro_sagemaker.py   # this branch
MaxTokensReachedException
tool ran: [] | invoke_endpoint calls: 1

Related Issues

Refs #5428

Documentation PR

None needed.

Type of Change

Bug fix

Testing

  • I ran hatch run prepare
    • Steps run directly.

Additional Details

Output
# strands-py, python 3.12, base 15ca21f15
$ pytest -q -n 4 tests/strands/models tests/strands/event_loop
1094 passed, 3 warnings, 9 errors   # this branch
1091 passed, 3 warnings, 9 errors   # base; the 9 errors are provider SDKs not installed
$ pytest -q -n 4 tests/strands/models tests/strands/event_loop   # sagemaker.py reverted, tests kept
FAILED tests/strands/models/test_sagemaker.py::TestSageMakerAIModel::test_stream_non_streaming_tool_call_stop_reason[length-max_tokens]
1 failed, 1093 passed, 3 warnings, 9 errors
$ ruff format --check src/strands/models/sagemaker.py tests/strands/models/test_sagemaker.py && ruff check src/strands/models/sagemaker.py tests/strands/models/test_sagemaker.py   # ruff 0.17.3
2 files already formatted
All checks passed!
$ mypy src/strands/models/sagemaker.py   # mypy 2.3.1
Success: no issues found in 1 source file
# not run: hatch test --all on 3.10, 3.11, 3.13, 3.14; a live SageMaker endpoint

Checklist

  • I have read the CONTRIBUTING document

  • I have reviewed and understand every line of code in this PR, and I can explain why it works

  • My change is focused and reasonably small; I have split unrelated work into separate PRs

  • I have added any necessary tests that prove my fix is effective or my feature works

  • I have updated the documentation accordingly

    • Nothing documented changes.
  • I have added an appropriate example to the documentation to outline the feature, or no new docs are needed

  • My changes generate no new warnings

  • Any dependent changes have been merged and published


By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@liramon2, stop beside a tool call still becomes tool_use. Right for your endpoints?

…f by the token limit

The non-streaming branch of SageMakerAIModel.stream replaced finish_reason
"length" with "tool_calls" whenever the message carried a tool call, so the
agent ran the tool on truncated arguments. The streaming branch already
passes "length" through, which maps to max_tokens.

Signed-off-by: Christopher Krah <48338417+chrikrah@users.noreply.github.com>
@chrikrah
chrikrah requested a review from a team as a code owner October 28, 2026 15:44
@chrikrah
chrikrah requested a review from notowen333 October 28, 2026 15:44
@github-actions github-actions Bot added python Pull requests that update python code bug Something isn't working area-model Related to models or model providers labels Oct 28, 2026
@github-actions github-actions Bot added complexity/high A touched function exceeds cognitive complexity 25; may be worth splitting size/xs labels Oct 30, 2026
@codecov

codecov Bot commented Oct 30, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@liramon2

Copy link
Copy Markdown
Contributor

Yes, stop beside another tool call should still run the tool and report tool_use.

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

area-model Related to models or model providers bug Something isn't working complexity/high A touched function exceeds cognitive complexity 25; may be worth splitting python Pull requests that update python code size/xs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants