You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Adds shared LlmRunSettings under providers/inference to map OpenResponses generation/loop-limit fields to Pydantic AI ModelSettings and UsageLimits. Includes unit tests and README updates. Not wired into handlers yet.
Note: This is not the only PR related to this issue.
If it is a core feature, I have added thorough tests.
Testing
uv run make format
uv run make verify
uv run make test-unit — tests/unit/providers/inference/test_llm_settings.py
Summary by CodeRabbit
New Features
Added support for configuring AI response generation options, reasoning and text preferences, and limits on tool calls and inference steps.
Responses-style request settings can now be carried into inference configuration, including reasoning effort and summary preferences. Unset options are omitted, while explicitly disabled options are preserved.
Reviewing files that changed from the base of the PR and between a05442a and c4b4cd8.
You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.
Use the checkbox below for a quick retry:
Trigger review
Walkthrough
The inference provider adds LlmRunSettings to map Responses request fields into model settings and usage limits. Unit tests cover field extraction, serialization, and limit behavior.
Changes
LLM run settings
Layer / File(s)
Summary
Settings model, conversion, and validation src/providers/inference/llm_settings.py, tests/unit/providers/inference/test_llm_settings.py, src/providers/inference/README.md, tests/unit/providers/inference/README.md
Adds request-setting extraction, model-settings serialization, and usage-limit conversion. Unit tests cover mapped fields, omitted values, explicit False, and limit defaults. Directory READMEs list the new module and tests.
The new settings resolver is not yet used by any handler, so merging it changes no runtime behavior today. Before it is wired in, the response-wide output-token limit should also be mapped to the run-wide usage limit.
Pre-merge checks: 7
Check name
Status
Explanation
Description Check
Check skipped - CodeRabbit’s high-level summary is enabled.
Title check
The title clearly identifies the implementation of the LLM settings resolver, which matches the main change.
Docstring Coverage
Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat…
Linked Issues check
Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check
Check skipped because no linked issues were found for this pull request.
Performance And Algorithmic Complexity
No meaningful performance regression is introduced. The new resolver processes a fixed set of 10 fields, uses one bounded Pydantic serialization call, and has no loops over request items, API calls, l…
Security And Secret Handling
The changed code adds a settings mapper and unit tests only. LlmRunSettings copies request values into typed fields and serializes them into ModelSettings/UsageLimits; it does not log, hardcode,…
Full details: Docstring Coverage
Explanation
Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 3 functions across 2 files. (2 skipped: 2 unsupported.)
Full details: Performance And Algorithmic Complexity
Explanation
No meaningful performance regression is introduced. The new resolver processes a fixed set of 10 fields, uses one bounded Pydantic serialization call, and has no loops over request items, API calls, list operations, unbounded state, or cache growth. The PR does not wire the resolver into handlers.
Full details: Security And Secret Handling
Explanation
The changed code adds a settings mapper and unit tests only. LlmRunSettings copies request values into typed fields and serializes them into ModelSettings/UsageLimits; it does not log, hardcode, expose, or execute secret-bearing values. No API endpoint, Kubernetes Secret, SQL/command/path operation, or response/logging path changes in the reviewed diff.
Finishing Touches
Generate unit tests (beta)
Create a new PR
Simplify code
Create a new PR
Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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.
Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.
Recent review details
Context from checks skipped due to timeout: 27
GitHub Check: Red Hat Konflux / rag-content-0-8-e2e-tests / lightspeed-stack-0-8
GitHub Check: mypy
GitHub Check: unit_tests (3.12)
GitHub Check: list_outdated_dependencies
GitHub Check: ruff
GitHub Check: integration_tests (3.12)
GitHub Check: unit_tests (3.14)
GitHub Check: pydocstyle
GitHub Check: integration_tests (3.13)
GitHub Check: Pylinter
GitHub Check: check_dependencies
GitHub Check: unit_tests (3.13)
GitHub Check: radon
GitHub Check: build-pr
GitHub Check: dispatch / Route
GitHub Check: integration_tests (3.14)
GitHub Check: spectral
GitHub Check: Pyright
GitHub Check: Red Hat Konflux / lightspeed-core-0-8-enterprise-contract / lightspeed-stack-0-8
GitHub Check: dispatch / Harness dispatch
GitHub Check: bandit
GitHub Check: shellcheck
GitHub Check: black
GitHub Check: check
GitHub Check: authorize / Check repository owner or member
GitHub Check: Red Hat Konflux / lightspeed-stack-0-8-e2e-tests / lightspeed-stack-0-8
Source excerpt: **Required**: Use pytest for all unit and integration tests **Forbidden**: Do not use unittest - pytest is the standard for this project Source excerpt: **Coverage**: Unit tests require 60% coverage, integration 10%
Source excerpt: **Required**: Use pytest for all unit and integration tests Source excerpt: **Forbidden**: Do not use unittest - pytest is the standard for this project Source excerpt: **Coverage**: Unit tests require 60% coverage, integrat...
Low-risk PR adding 2 new source files (LLM settings resolver + unit tests) and 2 README updates. Small scope (4 files, 296 lines), no protected paths, no security-sensitive paths, no CI or dependency changes. All files are net-new with no prior churn. Anchored to prior score of 1.
Low-risk PR adding 2 new source files (llm_settings resolver + unit tests) and 2 README updates. Small scope (4 files, 296 lines), no protected paths, no security-sensitive paths, no CI or dependency changes.
[missing-test]tests/unit/providers/inference/test_llm_settings.py — No test verifies that thinking=False (mapped from effort="none") survives the round-trip through to_model_settings(). The test test_from_responses_maps_reasoning_none_to_thinking_false only asserts settings.thinking is False but never calls to_model_settings(). Since exclude_none=True is used in model_dump, it is important to confirm that False (a falsy but non-None value) appears as {"thinking": False} in the output dict.
[test-inadequate]tests/unit/providers/inference/test_llm_settings.py:109 — The test test_to_usage_limits_preserves_default_request_limit hardcodes assert limits.request_limit == 50, coupling the test to pydantic-ai's internal default for UsageLimits.request_limit. If pydantic-ai changes that default in a future version, this test will break without any production code change.
[pattern-inconsistency]src/providers/inference/llm_settings.py:17 — The module-level constant _USAGE_EXCLUDE is missing the Final annotation required by project convention (AGENTS.md: "Use Final[type] as type hint for all constants"). Existing package constants follow this pattern (e.g. PROVIDER_CLASSES: Final[...] in registry.py). Additionally, ReasoningSummary and TextVerbosity type aliases are defined inline rather than in types.py, which is acceptable while only used locally.
Remediation: Add Final to the typing import and annotate: _USAGE_EXCLUDE: Final[set[str]] = {"max_tool_calls", "max_infer_iters"}
Earlier findings
src/providers/inference/llm_settings.py pattern-inconsistency — reclassified from info to low (finding now also covers missing Final annotation per project convention)
[missing-test]tests/unit/providers/inference/test_llm_settings.py — No test covers the end-to-end path where effort="none" produces thinking=False which then flows through to_model_settings() emitting {"thinking": False}. The existing test_from_responses_maps_reasoning_none_to_thinking_false test only verifies the attribute is False but never calls to_model_settings() to confirm it appears in the serialized output (since False is not None, it will survive exclude_none=True).
Remediation: Add a test that calls from_responses_request with effort='none' and then asserts to_model_settings() includes {"thinking": False}.
[test-inadequate]tests/unit/providers/inference/test_llm_settings.py:109 — The test test_to_usage_limits_preserves_default_request_limit hardcodes 50 as the expected default value of UsageLimits.request_limit. This couples the test to an implementation detail of pydantic-ai's UsageLimits class.
Remediation: Assert against UsageLimits().request_limit instead of the magic number 50.
Info
[pattern-inconsistency]src/providers/inference/llm_settings.py:14 — Module-level type aliases ReasoningSummary and TextVerbosity are defined in llm_settings.py, but the established pattern in this package is to define shared literals and type aliases in types.py. If these aliases will be consumed by other modules, consider moving them.
[design-direction]src/providers/inference/llm_settings.py — Placing LlmRunSettings under providers/inference/ is a reasonable fit: it bridges provider-level request parameters to the Pydantic AI run configuration. The module follows project conventions. Not yet wired into any handler, consistent with the stated incremental rollout.
Labels: New feature PR adding inference provider settings with tests
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
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.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
![fullsend-ai-review[bot]](https://avatars.githubusercontent.com/in/3478501?s=60&v=4){kind=small}
fullsend-ai-review
Bot
added
enhancement
Description
Adds shared
LlmRunSettingsunderproviders/inferenceto map OpenResponses generation/loop-limit fields to Pydantic AIModelSettingsandUsageLimits. Includes unit tests and README updates. Not wired into handlers yet.Note: This is not the only PR related to this issue.
Type of change
pyproject.toml+uv.lock]requirements.*.txtfor Konflux]Tools used to create PR
Related Tickets & Documents
Checklist before requesting a review
Testing
uv run make formatuv run make verifyuv run make test-unit—tests/unit/providers/inference/test_llm_settings.pySummary by CodeRabbit