OLS-3548 Omit temperature for models that deprecate it

[thoraxe]

Summary

• Adds temperature_supported boolean flag to ModelParameters (defaults to True for backward compatibility)
• The base LLMProvider strips temperature from LLM params when the model's config sets temperature_supported: false
• Fixes HTTP 400 errors from models like claude-sonnet-5 that have deprecated the temperature parameter

How to use

In olsconfig.yaml, set temperature_supported: false on any model that rejects temperature:

models:
  - name: anthropic.claude-sonnet-5
    parameters:
      temperature_supported: false

Models without this setting (or with temperature_supported: true) continue to receive temperature as before.

Test plan

• New unit tests verify temperature is stripped when temperature_supported=False
• New unit tests verify temperature is still present when temperature_supported=True (default)
• New unit tests verify caller-supplied temperature is also stripped when not supported
• All 98 existing LLM provider unit tests pass
• All 210 config model tests pass
• mypy type check passes on modified files

Summary by CodeRabbit

• New Features

  • Added configuration support to indicate whether a model accepts the temperature parameter.
  • For models that don’t support temperature, the app now omits it automatically from outgoing requests.

• Documentation

  • Updated the example configuration with guidance for disabling temperature support when needed.

• Tests

  • Expanded unit test coverage to verify temperature is stripped or retained correctly, including when callers supply a temperature value.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 27fbece2-af68-49e9-83bd-a5c52674ee4f

📥 Commits

Reviewing files that changed from the base of the PR and between 61d9a97 and fee421e.

📒 Files selected for processing (1)

• ols/src/llms/providers/provider.py

🚧 Files skipped from review as they are similar to previous changes (1)

• ols/src/llms/providers/provider.py

---

Walkthrough

Adds a model-level temperature_supported setting and updates provider parameter handling to omit temperature for models that do not support it. Bedrock tests cover supported and unsupported configurations.

Changes

Temperature Support Handling

Layer / File(s)	Summary
Temperature capability configuration ols/app/models/config.py, examples/olsconfig.yaml	Adds temperature_supported, defaulting to True, to model parameters and documents the option in the example configuration.
Provider override and Bedrock validation ols/src/llms/providers/provider.py, tests/unit/llms/providers/test_bedrock.py	Removes unsupported temperature parameters during provider overrides and tests default, supported, unsupported, and caller-supplied behavior.

Estimated code review effort: 2 (Simple) | ~10 minutes

Suggested reviewers: onmete, tisnik

🚥 Pre-merge checks | ✅ 15

✅ Passed checks (15 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly reflects the main change: omitting temperature for models that do not support it.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Stable And Deterministic Test Names	✅ Passed	The added tests use static, descriptive names and no It/Describe-style titles include dynamic data or generated identifiers.
Test Structure And Quality	✅ Passed	Repo uses pytest, not Ginkgo; the new Bedrock tests are isolated, fixture-based, and match existing provider-test style without cleanup or timeout concerns.
Microshift Test Compatibility	✅ Passed	No new Ginkgo e2e tests were added; the PR changes only config/provider code and pytest unit tests, with no MicroShift-unsupported OpenShift API usage.
Single Node Openshift (Sno) Test Compatibility	✅ Passed	No new Ginkgo e2e tests were added; the PR only changes Python config/provider code and unit tests, so there are no SNO topology assumptions to review.
Topology-Aware Scheduling Compatibility	✅ Passed	Only ols/src/llms/providers/provider.py changed, and it only strips temperature based on model config; no manifests, controllers, or scheduling constraints were added.
Ote Binary Stdout Contract	✅ Passed	The PR only touches config/model/provider logic and unit tests; no process-level entrypoints or stdout writes were added, and provider uses logger calls only.
Ipv6 And Disconnected Network Test Compatibility	✅ Passed	No new Ginkgo e2e tests were added; the only modified file is provider.py, so IPv4/disconnected-network checks don't apply.
No-Weak-Crypto	✅ Passed	PR only adds temperature_supported handling and tests; no MD5/SHA1/DES/RC4/3DES/Blowfish, custom crypto, or secret-compare code was introduced.
Container-Privileges	✅ Passed	PR changes only touch config/provider/tests; repo scan found no privileged, hostPID, hostNetwork, hostIPC, SYS_ADMIN, or allowPrivilegeEscalation settings in manifests.
No-Sensitive-Data-In-Logs	✅ Passed	The PR adds temp-support config and stripping logic only; no new logging of secrets, PII, hostnames, or customer data appears in the changed files.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[openshift-ci]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign bparees for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[coderabbitai]

🧹 Nitpick comments (1)

ols/src/llms/providers/provider.py (1)

382-392: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Simplify _model_supports_temperature by removing the dead getattr/None branch.

ModelConfig.parameters is declared as parameters: ModelParameters = ModelParameters(), so it always carries a value — the getattr fallback and the if params is None guard can never trigger. Direct access is cleaner and preserves full type information for mypy strict mode.

♻️ Proposed refactor

     def _model_supports_temperature(self) -> bool:
         """Check whether the current model supports the temperature parameter."""
         if self.provider_config is None:
             return True
         model_config = self.provider_config.models.get(self.model)
         if model_config is None:
             return True
-        params = getattr(model_config, "parameters", None)
-        if params is None:
-            return True
-        return params.temperature_supported
+        return model_config.parameters.temperature_supported

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@ols/src/llms/providers/provider.py` around lines 382 - 392, Update
_model_supports_temperature to access model_config.parameters directly and
return its temperature_supported value, removing the getattr fallback and params
None guard while preserving the existing provider_config and model_config None
handling.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@ols/src/llms/providers/provider.py`:
- Around line 382-392: Update _model_supports_temperature to access
model_config.parameters directly and return its temperature_supported value,
removing the getattr fallback and params None guard while preserving the
existing provider_config and model_config None handling.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 30b796f3-9d81-46ad-a570-ee0316ca07bf

📥 Commits

Reviewing files that changed from the base of the PR and between 6578cfb and 61d9a97.

📒 Files selected for processing (4)

• examples/olsconfig.yaml
• ols/app/models/config.py
• ols/src/llms/providers/provider.py
• tests/unit/llms/providers/test_bedrock.py

[thoraxe]

Addressed the CodeRabbit nitpick — removed the dead getattr/None guard in _model_supports_temperature. ModelConfig.parameters is declared with a default ModelParameters(), so it can never be None. Simplified to access model_config.parameters.temperature_supported directly.

All 22 bedrock provider tests pass.

[openshift-ci]

@thoraxe: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
ci/prow/e2e-ols-cluster	fee421e	link	true	/test e2e-ols-cluster
ci/prow/ols-evaluation	fee421e	link	true	/test ols-evaluation

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.