Skip to content

fix(ctl): honour temperature/max_tokens declared under spec.models - #56

Merged
jordanauge merged 1 commit into
mainfrom
fix/model-params-from-spec-models
Sep 11, 2026
Merged

jordanauge merged 1 commit into
mainfrom
fix/model-params-from-spec-models

Conversation

@jordanauge

Copy link
Copy Markdown
Collaborator

Problem

resolve_model_name() already accepts the model name from either spec.llm or spec.models[0], but engine_factory read temperature and max_tokens from spec.llm only.

Any manifest using the spec.models[] shape therefore silently fell back to the defaults (max_tokens=2000, temperature=0.7). A manifest declaring:

spec:
  models:
    - id: main
      model: vertex_ai/gemini-2.5-flash
      temperature: 0.0
      max_tokens: 8000

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 which reasoning_tokens: 1916 and text_tokens: 80 → the answer is truncated mid-sentence
  • in the worst case completion_tokens: 0 with finish_reason: "stop" → an empty completion, because reasoning consumed the whole (unexpectedly small) budget

Raising max_tokens in the manifest had no effect, which made this quite hard to diagnose: the declared value never reached the engine.

Change

  • _resolve_sampling_param(): symmetric spec.llmspec.models[0] lookup for temperature and max_tokens, mirroring what resolve_model_name already does for the model name. spec.llm still wins when both are set, and the existing defaults are unchanged when neither is.
  • _resolve_model_option() + LiveLlmEngine.reasoning_effort: optional reasoning_effort pass-through, so a manifest can bound internal reasoning on models that bill it against max_tokens. Defaults to None, so the request payload is unchanged unless a manifest opts in.

Compatibility

No behaviour change for manifests using spec.llm. Manifests using spec.models[] start getting the values they already declared.

- 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
jordanauge force-pushed the fix/model-params-from-spec-models branch from a4db131 to dc923d7 Compare September 11, 2026 13:59
@jordanauge
jordanauge merged commit 5469068 into main Sep 11, 2026
3 checks passed
@jordanauge
jordanauge deleted the fix/model-params-from-spec-models branch September 11, 2026 14:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant