fix: get_info() reports runtime-effective model, not packaged default - #81
Merged
Merged
Conversation
get_info() reported the packaged static model name from config/_models.py regardless of the runtime-configured model. The context_window and max_output_tokens fields in the same defaults dict were already correctly resolved against the effective model, but the model key itself was never corrected -- an internal inconsistency within the function. contracts/behaviors.md's Model Selection Policy requires respecting request.model > config["default_model"] > YAML priority everywhere; get_info() ignored that priority for defaults["model"] while honoring it for the window fields. Fix: set defaults["model"] = self._effective_default_model unconditionally, so the reported model is correct even on a cold model-info cache (matching how the window fields already handle the cold-cache case, just with the static fallback instead). Adds 4 tests to the existing TestRuntimeConfigOverride class in tests/test_behaviors.py covering: runtime override, YAML fallback (no regression), cold-cache correctness, and non-mutation of the shared lru_cached provider config. Generated with [Amplifier](https://github.com/microsoft/amplifier) Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
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
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
get_info()reports the packaged static model name fromconfig/_models.pyinstead of the runtime-configured effective model, violating the module's own documented model selection priority and creating an internal inconsistency within the function itself.The bug
contracts/behaviors.md(Model Selection Policy,MUST:1) states:get_info()already applies this priority correctly fordefaults["context_window"]anddefaults["max_output_tokens"]— both are resolved againstself._effective_default_model(which itself correctly implements the priority:self.config.get("default_model") or self._provider_config.defaults["model"]). Butdefaults["model"]was left untouched, so it always reported the packaged YAML default (claude-opus-4.5) regardless of what model was actually configured at runtime — even while the window fields, computed a few lines later in the same function, were correctly reporting values for the real effective model.Reproduction (with
config={"default_model": "mai-code-1.1-flash"}):This is unconditional — it reproduces identically whether the model-info cache is warm or cold, since the model field was never wired to the effective-model resolution at all.
User-visible impact: Consumers that read
get_info().defaults["model"]to determine the active model (e.g. to emit aprovider:resolve-style event driving a live model display) show the wrong model whenever a caller overridesdefault_modelat runtime (for example via a routing/config layer that assigns different models per sub-agent).The fix
One line, plus an explanatory comment, in
get_info():Placed unconditionally — before the model-info cache lookup, not inside the
if info is not None:branch that corrects the window fields — so the reported model is correct even when the model cache is cold. The cold-cache fallback behavior forcontext_window/max_output_tokensis intentionally unchanged.This brings the provider in line with sibling Amplifier providers (Anthropic, Chat Completions), which already report the runtime-resolved model from
get_info()rather than a static default.Tests
Added to the existing
TestRuntimeConfigOverrideclass intests/test_behaviors.py(the established home for Model Selection Policy tests):test_get_info_reports_runtime_default_model—config={"default_model": ...}is reflected inget_info().defaults["model"].test_get_info_reports_yaml_default_without_runtime_config— nodefault_modelconfigured still reports the YAML default (no regression).test_get_info_reports_runtime_model_on_cold_cache— with a cold model-info cache (lookup returnsNone), the model field is still correct, proving the fix applies outside theif info is not None:branch, while window-field cold-cache fallback stays unchanged.test_get_info_does_not_mutate_shared_cached_defaults— a second provider instance with a different config is unaffected by a prior instance'sget_info()call, sincecfg.defaultsis a process-wide@lru_cached singleton.No existing test asserted the previous (incorrect) behavior.
Quality gates
pytest tests/ -m "not live"— 1558 passed, 0 failedruff check amplifier_module_provider_github_copilot/ tests/— cleanpyright .— 0 errors, 0 warningsScope
Out of scope (unchanged, deliberately): the cold-cache fallback behavior for
context_window/max_output_tokens,get_model_info(),config/_models.py,FALLBACKS,resolve_effective_window, and the model cache implementation.Generated with Amplifier
Co-Authored-By: Amplifier 240397093+microsoft-amplifier@users.noreply.github.com