fix(ctl): honour temperature/max_tokens declared under spec.models - #56
Merged
Merged
Conversation
- Resolve model id, temperature, max_tokens, and reasoning_effort from spec.models[0] as the canonical agent manifest surface - Fall back to spec.llm only as a deprecated compat shim with a warning - Pass reasoning_effort through LiveLlmEngine to the provider payload - Mark spec.llm deprecated in schema; document reasoning_effort on models[] Signed-off-by: Jordan Augé <augjorda@cisco.com>
jordanauge
force-pushed
the
fix/model-params-from-spec-models
branch
from
September 11, 2026 13:59
a4db131 to
dc923d7
Compare
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.
Problem
resolve_model_name()already accepts the model name from eitherspec.llmorspec.models[0], butengine_factoryreadtemperatureandmax_tokensfromspec.llmonly.Any manifest using the
spec.models[]shape therefore silently fell back to the defaults (max_tokens=2000,temperature=0.7). A manifest declaring:was actually run at
max_tokens=2000/temperature=0.7, with no warning.Impact
This is easy to miss on short answers, but it is fatal on reasoning models, which bill internal reasoning against the same completion budget. Observed on a multi-round negotiation with Gemini 2.5 Flash:
completion_tokens: 1996, of whichreasoning_tokens: 1916andtext_tokens: 80→ the answer is truncated mid-sentencecompletion_tokens: 0withfinish_reason: "stop"→ an empty completion, because reasoning consumed the whole (unexpectedly small) budgetRaising
max_tokensin the manifest had no effect, which made this quite hard to diagnose: the declared value never reached the engine.Change
_resolve_sampling_param(): symmetricspec.llm→spec.models[0]lookup fortemperatureandmax_tokens, mirroring whatresolve_model_namealready does for the model name.spec.llmstill wins when both are set, and the existing defaults are unchanged when neither is._resolve_model_option()+LiveLlmEngine.reasoning_effort: optionalreasoning_effortpass-through, so a manifest can bound internal reasoning on models that bill it againstmax_tokens. Defaults toNone, so the request payload is unchanged unless a manifest opts in.Compatibility
No behaviour change for manifests using
spec.llm. Manifests usingspec.models[]start getting the values they already declared.