fix: pass provider= kwarg to context_manager.get_messages_for_request#11
Open
bkrabach wants to merge 1 commit into
Open
fix: pass provider= kwarg to context_manager.get_messages_for_request#11bkrabach wants to merge 1 commit into
bkrabach wants to merge 1 commit into
Conversation
…s_for_request The context-manager budget resolution cascade (see amplifier-module-context-simple:1157-1223) reads provider.get_info().defaults["context_window"] and ["max_output_tokens"] via the provider kwarg. Missing the kwarg silently disables kernel-side token budgeting — the cascade falls through to the context manager's generic max_tokens fallback (200k default), producing prompts that overflow smaller servers (llama.cpp, LM Studio, vLLM). loop-basic and loop-streaming pass provider= correctly at their equivalent sites. This brings loop-events to parity. Co-authored-by: Amplifier <amplifier@bkrabach>
hoopsomuah
pushed a commit
to anokye-labs/amplifier-module-loop-events
that referenced
this pull request
May 6, 2026
…ft#10) content_models.ThinkingContent has a .text attribute (distinct from message_models.ThinkingBlock which uses .thinking). The hasattr-based filter at both the main loop path and the max-iterations fallback was letting thinking text leak into final_response. Extracts a shared _extract_text_from_content helper and replaces both call sites. Same fix pattern as amplifier-module-loop-streaming PR #25 (merge df5c0e1) and amplifier-module-loop-basic PR microsoft#11. Adds regression tests verifying ThinkingContent is filtered at both code paths while TextBlock/TextContent pass through.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
context_manager.get_messages_for_request()withoutprovider=kwarg at two sites (__init__.py:158-159, :588-589), silently bypassing the duck-typed budget-resolution cascade in context-simple/persistent/managed.loop-events.loop-basicandloop-streamingpassprovider=correctly—this was a gaps-in-coverage issue.provider=providerat both call sites. Provider was already in local scope (assigned fromself._select_provider(providers)at line 142).Test Plan
test_tool_post_no_modify_uses_original) was already broken on mainGenerated with Amplifier
Co-Authored-By: Amplifier 240397093+microsoft-amplifier@users.noreply.github.com