Skip to content

fix(models): raise ContextWindowOverflowException for ollama/llama/mistral/writer#2958

Draft
gautamsirdeshmukh wants to merge 1 commit into
strands-agents:mainfrom
gautamsirdeshmukh:agent-tasks/2052
Draft

fix(models): raise ContextWindowOverflowException for ollama/llama/mistral/writer#2958
gautamsirdeshmukh wants to merge 1 commit into
strands-agents:mainfrom
gautamsirdeshmukh:agent-tasks/2052

Conversation

@gautamsirdeshmukh

Copy link
Copy Markdown
Contributor

Description

Related Issues

Documentation PR

Type of Change

Bug fix
New feature
Breaking change
Documentation update
Other (please describe):

Testing

How have you tested the change? Verify that the changes do not break functionality or introduce new warnings.

  • I ran hatch run prepare

Checklist

  • I have read the CONTRIBUTING document
  • I have reviewed and understand every line of code in this PR, including any generated by AI tools, 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
  • 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.

@github-actions github-actions Bot added size/m area-model Related to models or model providers area-context Session or context related bug Something isn't working python Pull requests that update python code strands-running labels Jun 25, 2026
@codecov

codecov Bot commented Jun 25, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

- System prompts
"""

OVERFLOW_MESSAGES = {

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.

Issue (verification): The substring matching is the crux of this fix — if these strings don't match what the provider actually returns, the except branch becomes silent dead code and the bug persists undetected. The unit tests assert against these same hardcoded strings, so they'd pass even if the strings are wrong.

Suggestion: Confirm these strings against real provider error text (an integ test that intentionally overflows the context, or a captured real error in the PR description). Worth noting where each string came from, since several read like reasonable guesses (e.g. "input too large", "too many tokens").

"model's maximum context limit",
"is longer than the model's context length",
"prompt is too long",
"too many tokens",

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.

Issue: This OVERFLOW_MESSAGES set is byte-for-byte identical to the one added in llamaapi.py. Two copies of the same substring list will drift over time as one provider's error strings are tuned and the other is forgotten.

Suggestion: If LlamaAPI and Writer genuinely share the same OpenAI-compatible error vocabulary, consider hoisting the shared strings to a single module-level constant (mirroring how bedrock.py and openai.py keep these lists as module constants) rather than duplicating per class.

@github-actions

Copy link
Copy Markdown
Contributor

Assessment: Comment

Solid, well-targeted fix that follows the established provider pattern (Anthropic's OVERFLOW_MESSAGES set, Bedrock/OpenAI substring lists) and is backed by passing unit tests — including nice coverage of Ollama's lazy request surfacing overflow both at chat() and mid-iteration. A few things worth addressing before merge:

Review themes
  • Coverage gap: The fix only touches stream(); structured_output() in Ollama/Mistral/Writer calls the API directly and remains unhandled, leaving that code path exactly as broken as the original issue describes.
  • Verification risk: The whole fix hinges on substring matching. With no integ test against real provider errors, a wrong string silently becomes dead code — and the unit tests, which assert on the same hardcoded strings, won't catch it.
  • Duplication: LlamaAPI and Writer carry identical OVERFLOW_MESSAGES sets that will drift independently.
  • Docs: Modified stream() docstrings don't note the new ContextWindowOverflowException in Raises:.
  • PR hygiene: Description is empty and doesn't link issue bug: Ollama, Mistral, LlamaAPI, and Writer providers missing ContextWindowOverflowException #2052 — worth filling in for reviewer context.

Nice work extending the recovery path to four more providers — the Ollama lazy-iteration handling in particular is a thoughtful touch.

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

Labels

area-context Session or context related area-model Related to models or model providers bug Something isn't working python Pull requests that update python code size/m

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants