Add rlsapi v1 infer request/response models #896

major · 2025-12-09T15:11:02Z

Description

This work is in preparation for adding the rlsapi v1 API endpoints from RHEL Lightspeed's production stateless API.

Type of change

Tools used to create PR

Identify any AI code assistants used in this PR (for transparency and review context)

Assisted-by: Claude

Related Tickets & Documents

Related Issue #
Closes #

Checklist before requesting a review

I have performed a self-review of my code.
PR has passed all pre-merge test jobs.
If it is a core feature, I have added thorough tests.

Testing

Provided tests run just fine with make verify

Summary by CodeRabbit

New Features
- Added support for RHEL Lightspeed rlsapi v1 inference, including request/response schemas and an action for rlsapi v1 inference.
- Request models include context, attachments, system info and enforce trimmed, non-empty questions; extra fields are rejected; example response schema provided.
Tests
- Added comprehensive unit tests for rlsapi v1 request/response validation, aliasing, schema examples, serialization roundtrips, and extra-field rejection.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

openshift-ci · 2025-12-09T15:11:13Z

Hi @major. Thanks for your PR.

I'm waiting for a github.com member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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.

coderabbitai · 2025-12-09T15:11:19Z

Walkthrough

Adds RHEL Lightspeed (rlsapi) v1 support by introducing Pydantic request and response models, adding an Action enum member for rlsapi inference, and adding unit tests plus a minor test import reorder.

Changes

Cohort / File(s)	Summary
Config Extension `src/models/config.py`	Added enum member `RLSAPI_V1_INFER = "rlsapi_v1_infer"` in `Action`.
RlsAPI v1 Package Init `src/models/rlsapi/__init__.py`	Added package init with module docstring for rlsapi v1 models.
RlsAPI v1 Request Models `src/models/rlsapi/requests.py`	New Pydantic models: `RlsapiV1Attachment` (`contents`, `mimetype`), `RlsapiV1Terminal` (`output`), `RlsapiV1SystemInfo` (`os`, `version`, `arch`, `system_id` with alias `id`, populate_by_name enabled), `RlsapiV1CLA` (`nevra`, `version`), `RlsapiV1Context` (nested fields with `default_factory`), and `RlsapiV1InferRequest` (`question` with validator that strips and rejects empty/whitespace, `context`, `skip_rag`). Models forbid extra fields.
RlsAPI v1 Response Models `src/models/rlsapi/responses.py`	New Pydantic response models: `RlsapiV1InferData` (`text`, optional `request_id`) and `RlsapiV1InferResponse` (extends `AbstractSuccessfulResponse`, includes `data`, forbids extras, supplies JSON schema example).
Tests & Test Init `tests/unit/authentication/test_api_key_token.py`, `tests/unit/models/rlsapi/__init__.py`, `tests/unit/models/rlsapi/test_requests.py`, `tests/unit/models/rlsapi/test_responses.py`	Minor import reorder in an authentication test; added test package init; comprehensive unit tests for rlsapi v1 request/response models covering defaults, aliasing, extra-field rejection, validation (including `question` trimming/whitespace), schema examples, and serialization roundtrips.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Inspect RlsapiV1InferRequest.validate_question for trimming and precise error messages.
Verify default_factory usage in RlsapiV1Context avoids shared mutable defaults.
Confirm RlsapiV1SystemInfo alias (id ↔ system_id) works with populate_by_name.
Check that response json_schema_extra example aligns with model fields and types.

Suggested reviewers

tisnik

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title directly and concisely summarizes the main change: adding rlsapi v1 infer request and response models.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✨ Finishing touches

📝 Generate docstrings

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment

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

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

coderabbitai

Actionable comments posted: 0

🧹 Nitpick comments (6)

tests/unit/authentication/test_api_key_token.py (1)
74-76: Use the detail local variable consistently

You bind detail = exc_info.value.detail and check its type, but then index exc_info.value.detail again instead of the local variable. For consistency and a tiny readability win, you can reuse detail:
-    detail = exc_info.value.detail
-    assert isinstance(detail, dict)
-    assert exc_info.value.detail["cause"] == "No Authorization header found"
+    detail = exc_info.value.detail
+    assert isinstance(detail, dict)
+    assert detail["cause"] == "No Authorization header found"
tests/unit/models/rlsapi/test_responses.py (1)
90-109: Consider simplifying the example count assertion.

The expected_count variable is computed but only asserted to equal 1 at the end without being used for any intermediate comparison. This can be simplified:
     def test_openapi_response(self) -> None:
         """Test RlsapiV1InferResponse.openapi_response() method."""
-        schema = RlsapiV1InferResponse.model_json_schema()
-        model_examples = schema.get("examples", [])
-        expected_count = len(model_examples)
-
         result = RlsapiV1InferResponse.openapi_response()
         assert result["description"] == "Successful response"
         assert result["model"] == RlsapiV1InferResponse
         assert "content" in result
         assert "application/json" in result["content"]
         assert "example" in result["content"]["application/json"]

         example = result["content"]["application/json"]["example"]
         assert "data" in example
         assert "text" in example["data"]
         assert "request_id" in example["data"]
-
-        # Verify example count matches schema examples count (should be 1)
-        assert expected_count == 1
The schema example count is already tested in test_json_schema_example (lines 111-122), so this assertion is redundant.
tests/unit/models/rlsapi/test_requests.py (2)
35-39: Reduce coupling to exact Pydantic error messages in match= assertions

Many pytest.raises(ValidationError, match="...") checks rely on the exact error text emitted by Pydantic (e.g., "Extra inputs are not permitted" and "String should have at least 1 character"). That wording can change between Pydantic versions, making these tests brittle even when behavior is still correct.

Consider instead asserting on the error structure via exc_info.value.errors() or using a looser regex (e.g., only matching a stable fragment), for example:
with pytest.raises(ValidationError) as exc_info:
    RlsapiV1InferRequest(question="")

errors = exc_info.value.errors()
# Adjust the expected type/fields to match your pinned Pydantic version
assert errors[0]["type"] == "string_too_short"
Likewise, for extra fields you could assert on the error type rather than the full message, and drop match=. This will make the tests more robust to library upgrades.

Please confirm the exact error codes/structure for your pinned Pydantic version and adjust the assertions accordingly.

Also applies to: 54-58, 89-93, 113-117, 158-162, 196-198, 203-205, 213-219

84-88: Optional: add a test covering system_id= input alongside the id alias

You currently verify that the alias id= populates system_id. Given populate_by_name=True on the model, it may be worth adding a small test that RlsapiV1SystemInfo(system_id="foo") also works as expected, to fully cover both input paths for this field.
src/models/rlsapi/requests.py (2)
1-3: Consider adding a module-level logger to match src logging guidelines

Project guidelines for src/**/*.py mention using the logging module with a logger = logging.getLogger(__name__) pattern. Even if this module doesn’t currently log, you may want to align with that convention for consistency and future debugging:
import logging
from pydantic import BaseModel, Field, field_validator

logger = logging.getLogger(__name__)
If other model-only modules in this package omit logging, feel free to keep it consistent with the prevailing pattern instead.

129-163: Minor docstring clarification and validate_question cleanup

Two small nits here:

The docstring describes context as “Optional context”, but the field type is RlsapiV1Context with a default_factory, so from the model’s perspective it’s always present and never None. To avoid confusion, either:

Reword to drop “Optional” (e.g., “Context with system info, terminal output, etc.; omitted fields fall back to defaults”), or

Change the type to RlsapiV1Context | None with an appropriate default if you truly want None to be valid.

In validate_question, you call value.strip() twice. Storing the stripped string once is a bit clearer and avoids the duplicate work:
@field_validator("question")
@classmethod
def validate_question(cls, value: str) -> str:
    """Validate question is not empty or whitespace-only."""
    stripped = value.strip()
    if not stripped:
        raise ValueError("Question cannot be empty or whitespace-only")
    return stripped
These don’t affect behavior but improve clarity and maintainability.

Also applies to: 167-183

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 5cb5c96 and 4b6519c.

📒 Files selected for processing (8)

src/models/config.py (1 hunks)
src/models/rlsapi/__init__.py (1 hunks)
src/models/rlsapi/requests.py (1 hunks)
src/models/rlsapi/responses.py (1 hunks)
tests/unit/authentication/test_api_key_token.py (3 hunks)
tests/unit/models/rlsapi/__init__.py (1 hunks)
tests/unit/models/rlsapi/test_requests.py (1 hunks)
tests/unit/models/rlsapi/test_responses.py (1 hunks)

🧰 Additional context used

📓 Path-based instructions (6)

src/**/*.py

📄 CodeRabbit inference engine (CLAUDE.md)

src/**/*.py: Use absolute imports for internal modules in LCS project (e.g., from auth import get_auth_dependency)
All modules must start with descriptive docstrings explaining their purpose
Use logger = logging.getLogger(__name__) pattern for module logging
All functions must include complete type annotations for parameters and return types, using modern syntax (str | int) and Optional[Type] or Type | None
All functions must have docstrings with brief descriptions following Google Python docstring conventions
Function names must use snake_case with descriptive, action-oriented names (get_, validate_, check_)
Avoid in-place parameter modification anti-patterns; return new data structures instead of modifying input parameters
Use async def for I/O operations and external API calls
All classes must include descriptive docstrings explaining their purpose following Google Python docstring conventions
Class names must use PascalCase with descriptive names and standard suffixes: Configuration for config classes, Error/Exception for exceptions, Resolver for strategy patterns, Interface for abstract base classes
Abstract classes must use ABC with @abstractmethod decorators
Include complete type annotations for all class attributes in Python classes
Use import logging and module logger pattern with standard log levels: debug, info, warning, error

Files:

src/models/rlsapi/responses.py
src/models/config.py
src/models/rlsapi/requests.py
src/models/rlsapi/__init__.py

src/models/**/*.py

📄 CodeRabbit inference engine (CLAUDE.md)

src/models/**/*.py: Use @field_validator and @model_validator for custom validation in Pydantic models
Pydantic configuration classes must extend ConfigurationBase; data models must extend BaseModel

Files:

src/models/rlsapi/responses.py
src/models/config.py
src/models/rlsapi/requests.py
src/models/rlsapi/__init__.py

tests/{unit,integration}/**/*.py

📄 CodeRabbit inference engine (CLAUDE.md)

tests/{unit,integration}/**/*.py: Use pytest for all unit and integration tests; do not use unittest framework
Unit tests must achieve 60% code coverage; integration tests must achieve 10% coverage

Files:

tests/unit/models/rlsapi/test_responses.py
tests/unit/models/rlsapi/test_requests.py
tests/unit/models/rlsapi/__init__.py
tests/unit/authentication/test_api_key_token.py

tests/**/*.py

📄 CodeRabbit inference engine (CLAUDE.md)

Use pytest-mock with AsyncMock objects for mocking in tests

Files:

tests/unit/models/rlsapi/test_responses.py
tests/unit/models/rlsapi/test_requests.py
tests/unit/models/rlsapi/__init__.py
tests/unit/authentication/test_api_key_token.py

src/models/config.py

📄 CodeRabbit inference engine (CLAUDE.md)

src/models/config.py: All configuration must use Pydantic models extending ConfigurationBase with extra="forbid" to reject unknown fields
Use type hints Optional[FilePath], PositiveInt, SecretStr for Pydantic configuration models

Files:

src/models/config.py

src/**/__init__.py

📄 CodeRabbit inference engine (CLAUDE.md)

Package __init__.py files must contain brief package descriptions

Files:

src/models/rlsapi/__init__.py

🧠 Learnings (1)

📚 Learning: 2025-11-24T16:58:04.410Z

Learnt from: CR
Repo: lightspeed-core/lightspeed-stack PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:58:04.410Z
Learning: Applies to src/**/__init__.py : Package `__init__.py` files must contain brief package descriptions

Applied to files:

tests/unit/models/rlsapi/__init__.py
src/models/rlsapi/__init__.py

🧬 Code graph analysis (1)

tests/unit/models/rlsapi/test_responses.py (2)

src/models/rlsapi/responses.py (2)

RlsapiV1InferData (8-27)

RlsapiV1InferResponse (30-54)

src/models/responses.py (1)

AbstractSuccessfulResponse (43-60)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)

GitHub Check: E2E: library mode / ci
GitHub Check: E2E: server mode / azure
GitHub Check: E2E: library mode / azure
GitHub Check: build-pr
GitHub Check: E2E: server mode / ci

🔇 Additional comments (10)

tests/unit/authentication/test_api_key_token.py (2)

6-6: Import reordering is fine

Reordering imported names on this line is purely stylistic and keeps the import list tidy; no behavior impact.

99-101: Updated detail assertions look good

Capturing detail, asserting it’s a dict, and then indexing detail["cause"] is clear and robust for this failure case; matches the intended structured error payload.

src/models/config.py (1)

807-808: LGTM!

The new RLSAPI_V1_INFER action follows the established naming conventions and includes a clear comment explaining its purpose for RHEL Lightspeed compatibility.

tests/unit/models/rlsapi/__init__.py (1)

1-1: LGTM!

Package init file contains the required brief description as per coding guidelines.

src/models/rlsapi/__init__.py (1)

1-1: LGTM!

Package init file contains the required brief description as per coding guidelines.

src/models/rlsapi/responses.py (1)

1-54: LGTM!

Well-structured Pydantic models that correctly extend BaseModel for data models (per coding guidelines). The models include:

Proper module and class docstrings with Attributes sections

Complete type annotations with modern syntax

extra="forbid" configuration for strict validation

Appropriate examples for OpenAPI schema generation

tests/unit/models/rlsapi/test_responses.py (3)

1-44: LGTM!

Comprehensive test coverage for RlsapiV1InferData including:

Minimal and full construction scenarios

Required field validation

Extra fields rejection

Tests use pytest correctly with proper docstrings and type annotations as per coding guidelines.

46-89: LGTM!

Thorough test coverage for RlsapiV1InferResponse construction, validation, and inheritance verification.

124-136: LGTM!

Good serialization roundtrip test ensuring the model can be properly serialized to JSON and deserialized back.

tests/unit/models/rlsapi/test_requests.py (1)

17-249: Comprehensive coverage of v1 request models looks good

The tests exercise defaults, aliasing, nested contexts, validators, and JSON round-trips in a clear, pytest-style way. This should give good confidence in the new request models’ behavior.

coderabbitai

Actionable comments posted: 0

🧹 Nitpick comments (3)

src/models/rlsapi/requests.py (3)

93-124: Confirm whether attachments should support multiple items

attachments is plural but typed as a single RlsapiV1Attachment with a non-list default. If the rlsapi v1 spec allows multiple attachments in a single request, you may want list[RlsapiV1Attachment] (with an empty-list default) instead of a single object. If the API guarantees at most one attachment, the current shape is fine but you might consider a singular field name for clarity.

149-163: Question validation is correct; consider simplifying whitespace handling

The combination of min_length=1 on question and the custom validate_question that strips and rejects whitespace-only strings is logically sound. You could optionally simplify by:

Dropping min_length=1 and relying solely on the validator, or

Adding strip_whitespace=True on the field and letting min_length=1 enforce non-emptiness, keeping the validator only if you want a custom error message.

Not required, but it would make the validation behavior slightly more predictable and centralized.

1-3: Optional: align with project-wide logger pattern

Coding guidelines for src/**/*.py mention importing logging and defining logger = logging.getLogger(__name__). This module currently doesn’t log anything, so it’s fine as-is, but if you want strict consistency with that guideline you could add the standard logger boilerplate now for future use.

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4b6519c and edb41cc.

📒 Files selected for processing (8)

src/models/config.py (1 hunks)
src/models/rlsapi/__init__.py (1 hunks)
src/models/rlsapi/requests.py (1 hunks)
src/models/rlsapi/responses.py (1 hunks)
tests/unit/authentication/test_api_key_token.py (3 hunks)
tests/unit/models/rlsapi/__init__.py (1 hunks)
tests/unit/models/rlsapi/test_requests.py (1 hunks)
tests/unit/models/rlsapi/test_responses.py (1 hunks)

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

src/models/config.py
tests/unit/models/rlsapi/init.py
src/models/rlsapi/responses.py
src/models/rlsapi/init.py
tests/unit/models/rlsapi/test_requests.py

🧰 Additional context used

📓 Path-based instructions (4)

tests/{unit,integration}/**/*.py

📄 CodeRabbit inference engine (CLAUDE.md)

tests/{unit,integration}/**/*.py: Use pytest for all unit and integration tests; do not use unittest framework
Unit tests must achieve 60% code coverage; integration tests must achieve 10% coverage

Files:

tests/unit/authentication/test_api_key_token.py
tests/unit/models/rlsapi/test_responses.py

tests/**/*.py

📄 CodeRabbit inference engine (CLAUDE.md)

Use pytest-mock with AsyncMock objects for mocking in tests

Files:

tests/unit/authentication/test_api_key_token.py
tests/unit/models/rlsapi/test_responses.py

src/**/*.py

📄 CodeRabbit inference engine (CLAUDE.md)

src/**/*.py: Use absolute imports for internal modules in LCS project (e.g., from auth import get_auth_dependency)
All modules must start with descriptive docstrings explaining their purpose
Use logger = logging.getLogger(__name__) pattern for module logging
All functions must include complete type annotations for parameters and return types, using modern syntax (str | int) and Optional[Type] or Type | None
All functions must have docstrings with brief descriptions following Google Python docstring conventions
Function names must use snake_case with descriptive, action-oriented names (get_, validate_, check_)
Avoid in-place parameter modification anti-patterns; return new data structures instead of modifying input parameters
Use async def for I/O operations and external API calls
All classes must include descriptive docstrings explaining their purpose following Google Python docstring conventions
Class names must use PascalCase with descriptive names and standard suffixes: Configuration for config classes, Error/Exception for exceptions, Resolver for strategy patterns, Interface for abstract base classes
Abstract classes must use ABC with @abstractmethod decorators
Include complete type annotations for all class attributes in Python classes
Use import logging and module logger pattern with standard log levels: debug, info, warning, error

Files:

src/models/rlsapi/requests.py

src/models/**/*.py

📄 CodeRabbit inference engine (CLAUDE.md)

src/models/**/*.py: Use @field_validator and @model_validator for custom validation in Pydantic models
Pydantic configuration classes must extend ConfigurationBase; data models must extend BaseModel

Files:

src/models/rlsapi/requests.py

🧬 Code graph analysis (1)

tests/unit/models/rlsapi/test_responses.py (2)

src/models/rlsapi/responses.py (2)

RlsapiV1InferData (8-27)

RlsapiV1InferResponse (30-54)

src/models/responses.py (1)

AbstractSuccessfulResponse (43-60)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)

GitHub Check: build-pr
GitHub Check: E2E: server mode / azure
GitHub Check: E2E: library mode / ci
GitHub Check: E2E: server mode / ci
GitHub Check: E2E: library mode / azure

🔇 Additional comments (7)

tests/unit/authentication/test_api_key_token.py (1)

74-76: LGTM! Improved error detail validation.

The updated assertions now properly validate the structured error response format, checking that detail is a dictionary with a cause key. This is more precise than checking a flat string and aligns with best practices for API error responses.

Also applies to: 99-101

tests/unit/models/rlsapi/test_responses.py (5)

1-1: The pylint disable is acceptable for Pydantic model tests.

The no-member disable is standard for Pydantic model tests since pylint cannot statically detect model attributes added via metaclasses. This is an acceptable practice.

21-33: LGTM! Well-structured fixtures.

The fixtures are properly configured with clear names, type hints, and follow pytest best practices. Good use of fixture composition where sample_response_fixture depends on sample_data_fixture.

41-68: LGTM! Excellent use of parametrized tests.

The parametrized tests effectively cover common validation patterns across both models, reducing code duplication while maintaining comprehensive coverage. The use of pytest.raises with match patterns ensures precise error validation.

76-88: LGTM! Comprehensive model construction tests.

The test cases properly cover both minimal construction (required fields only) and full construction (all fields), including validation that the optional request_id field defaults to None.

96-159: LGTM! Comprehensive response model test coverage.

The test suite provides excellent coverage of all critical aspects:

Model construction (both direct and dict-based)

Inheritance verification

OpenAPI schema generation with examples

JSON schema validation

Serialization/deserialization roundtrip

The use of type: ignore[arg-type] on line 110 is appropriate since Pydantic performs dict-to-model coercion that type checkers don't recognize.

src/models/rlsapi/requests.py (1)

6-166: Overall: models are well-structured and ready to support rlsapi v1 /infer

The request-side model set looks coherent: field types and defaults are explicit, nested context composition is clear, extra fields are forbidden, and docstrings/attribute descriptions are in good shape. This should integrate cleanly with the planned v1 infer endpoint.

tisnik · 2025-12-09T16:09:55Z

/ok-to-test

tisnik

one nit, but LGTM instread

tisnik · 2025-12-09T16:12:17Z

src/models/rlsapi/requests.py

+        examples=["text/plain", "application/json"],
+    )
+
+    model_config = {"extra": "forbid"}


nit:

define:

class ConfigurationBase(BaseModel): """Base class for all configuration models that rejects unknown fields.""" model_config = ConfigDict(extra="forbid")

then derive your Pydantic class not from BaseModel but from ConfigurationBase (or name it differently)

I missed that! Thank you.

This work is in preparation for adding the rlsapi v1 API endpoints from RHEL Lightspeed's production stateless API. Signed-off-by: Major Hayden <major@redhat.com>

coderabbitai

Actionable comments posted: 0

🧹 Nitpick comments (1)

tests/unit/authentication/test_api_key_token.py (1)

99-101: LGTM: Consistent error detail validation.

Good use of the same defensive pattern as in test_api_key_with_token_auth_dependency_no_token.

Optional observation: The error detail structure appears inconsistent across different error cases—Line 126 expects a plain string ("Invalid API Key"), while these tests expect a dict with a "cause" key. While this is outside the scope of this PR, consider standardizing the error response structure for better API consistency.

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between edb41cc and bedc67c.

📒 Files selected for processing (8)

src/models/config.py (1 hunks)
src/models/rlsapi/__init__.py (1 hunks)
src/models/rlsapi/requests.py (1 hunks)
src/models/rlsapi/responses.py (1 hunks)
tests/unit/authentication/test_api_key_token.py (3 hunks)
tests/unit/models/rlsapi/__init__.py (1 hunks)
tests/unit/models/rlsapi/test_requests.py (1 hunks)
tests/unit/models/rlsapi/test_responses.py (1 hunks)

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

tests/unit/models/rlsapi/test_requests.py
src/models/rlsapi/init.py
tests/unit/models/rlsapi/init.py
src/models/rlsapi/responses.py
src/models/rlsapi/requests.py

🧰 Additional context used

📓 Path-based instructions (5)

tests/{unit,integration}/**/*.py

📄 CodeRabbit inference engine (CLAUDE.md)

tests/{unit,integration}/**/*.py: Use pytest for all unit and integration tests; do not use unittest framework
Unit tests must achieve 60% code coverage; integration tests must achieve 10% coverage

Files:

tests/unit/authentication/test_api_key_token.py
tests/unit/models/rlsapi/test_responses.py

tests/**/*.py

📄 CodeRabbit inference engine (CLAUDE.md)

Use pytest-mock with AsyncMock objects for mocking in tests

Files:

tests/unit/authentication/test_api_key_token.py
tests/unit/models/rlsapi/test_responses.py

src/**/*.py

📄 CodeRabbit inference engine (CLAUDE.md)

src/**/*.py: Use absolute imports for internal modules in LCS project (e.g., from auth import get_auth_dependency)
All modules must start with descriptive docstrings explaining their purpose
Use logger = logging.getLogger(__name__) pattern for module logging
All functions must include complete type annotations for parameters and return types, using modern syntax (str | int) and Optional[Type] or Type | None
All functions must have docstrings with brief descriptions following Google Python docstring conventions
Function names must use snake_case with descriptive, action-oriented names (get_, validate_, check_)
Avoid in-place parameter modification anti-patterns; return new data structures instead of modifying input parameters
Use async def for I/O operations and external API calls
All classes must include descriptive docstrings explaining their purpose following Google Python docstring conventions
Class names must use PascalCase with descriptive names and standard suffixes: Configuration for config classes, Error/Exception for exceptions, Resolver for strategy patterns, Interface for abstract base classes
Abstract classes must use ABC with @abstractmethod decorators
Include complete type annotations for all class attributes in Python classes
Use import logging and module logger pattern with standard log levels: debug, info, warning, error

Files:

src/models/config.py

src/models/config.py

📄 CodeRabbit inference engine (CLAUDE.md)

src/models/config.py: All configuration must use Pydantic models extending ConfigurationBase with extra="forbid" to reject unknown fields
Use type hints Optional[FilePath], PositiveInt, SecretStr for Pydantic configuration models

Files:

src/models/config.py

src/models/**/*.py

📄 CodeRabbit inference engine (CLAUDE.md)

src/models/**/*.py: Use @field_validator and @model_validator for custom validation in Pydantic models
Pydantic configuration classes must extend ConfigurationBase; data models must extend BaseModel

Files:

src/models/config.py

🧬 Code graph analysis (1)

tests/unit/models/rlsapi/test_responses.py (2)

src/models/rlsapi/responses.py (2)

RlsapiV1InferData (9-26)

RlsapiV1InferResponse (29-53)

src/models/responses.py (1)

AbstractSuccessfulResponse (43-60)

🪛 Gitleaks (8.30.0)

src/models/config.py

[high] 808-808: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)

GitHub Check: E2E: server mode / ci
GitHub Check: E2E: library mode / ci
GitHub Check: E2E: library mode / azure
GitHub Check: E2E: server mode / azure
GitHub Check: build-pr

🔇 Additional comments (8)

src/models/config.py (1)

807-808: LGTM! New action enum member added correctly.

The new RLSAPI_V1_INFER action enum member follows the existing naming conventions and is properly documented. The static analysis warning about a "Generic API Key" on line 808 is a false positive—this is simply an enum value string identifier, not a secret or credential.

tests/unit/models/rlsapi/test_responses.py (5)

1-14: LGTM! Imports and module setup are correct.

The imports follow the coding guidelines with absolute imports for internal modules and proper type annotations.

21-34: LGTM! Well-structured fixtures with proper typing.

The fixtures follow pytest best practices with descriptive names, complete type annotations, and clear docstrings.

41-68: LGTM! Excellent use of parameterized tests to reduce duplication.

The parameterized tests efficiently verify that both RlsapiV1InferData and RlsapiV1InferResponse properly reject extra fields and enforce required fields, following DRY principles.

76-89: LGTM! Comprehensive tests for RlsapiV1InferData.

The tests properly validate both minimal construction (required fields only) and full construction (all fields), ensuring the model behaves correctly in different scenarios.

96-159: LGTM! Thorough test coverage for RlsapiV1InferResponse.

This test class provides excellent coverage:

Validates inheritance from AbstractSuccessfulResponse

Tests dict-based construction for API flexibility

Verifies OpenAPI schema generation for proper API documentation

Tests JSON serialization round-trip to ensure data integrity

The test quality aligns well with the coding guidelines and demonstrates good understanding of the model's requirements.

tests/unit/authentication/test_api_key_token.py (2)

6-6: LGTM: Import order update.

Minor formatting change with no functional impact.

74-76: LGTM: Improved error detail validation.

The added isinstance check before accessing the dict key is good defensive programming. This explicitly validates the error response structure and will catch any unintended changes to the error format.

tisnik

LGTM

tisnik · 2025-12-09T16:57:21Z

/ok-to-test

openshift-ci bot added the needs-ok-to-test label Dec 9, 2025

coderabbitai bot reviewed Dec 9, 2025

View reviewed changes

major force-pushed the rlsapi-models branch from 4b6519c to edb41cc Compare December 9, 2025 15:39

coderabbitai bot reviewed Dec 9, 2025

View reviewed changes

openshift-ci bot added ok-to-test and removed needs-ok-to-test labels Dec 9, 2025

tisnik reviewed Dec 9, 2025

View reviewed changes

Add rlsapi v1 infer request/response models

bedc67c

This work is in preparation for adding the rlsapi v1 API endpoints from RHEL Lightspeed's production stateless API. Signed-off-by: Major Hayden <major@redhat.com>

major force-pushed the rlsapi-models branch from edb41cc to bedc67c Compare December 9, 2025 16:26

coderabbitai bot reviewed Dec 9, 2025

View reviewed changes

tisnik approved these changes Dec 9, 2025

View reviewed changes

tisnik merged commit e897ab0 into lightspeed-core:main Dec 9, 2025
21 of 25 checks passed

coderabbitai bot mentioned this pull request Jan 15, 2026

fix(rlsapi): add error handling, metrics, and system context #1006

Merged

7 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Add rlsapi v1 infer request/response models #896

Add rlsapi v1 infer request/response models #896

Uh oh!

major commented Dec 9, 2025 •

edited by coderabbitai bot

Loading

Uh oh!

openshift-ci bot commented Dec 9, 2025

Uh oh!

coderabbitai bot commented Dec 9, 2025 •

edited

Loading

Uh oh!

coderabbitai bot left a comment

Uh oh!

coderabbitai bot left a comment

Uh oh!

tisnik commented Dec 9, 2025

Uh oh!

tisnik left a comment

Uh oh!

tisnik Dec 9, 2025

Uh oh!

major Dec 9, 2025

Uh oh!

coderabbitai bot left a comment

Uh oh!

tisnik left a comment

Uh oh!

tisnik commented Dec 9, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Add rlsapi v1 infer request/response models #896

Add rlsapi v1 infer request/response models #896

Uh oh!

Conversation

major commented Dec 9, 2025 • edited by coderabbitai bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Type of change

Tools used to create PR

Related Tickets & Documents

Checklist before requesting a review

Testing

Summary by CodeRabbit

Uh oh!

openshift-ci bot commented Dec 9, 2025

Uh oh!

coderabbitai bot commented Dec 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Suggested reviewers

Pre-merge checks and finishing touches

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

tisnik commented Dec 9, 2025

Uh oh!

tisnik left a comment

Choose a reason for hiding this comment

Uh oh!

tisnik Dec 9, 2025

Choose a reason for hiding this comment

Uh oh!

major Dec 9, 2025

Choose a reason for hiding this comment

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

tisnik left a comment

Choose a reason for hiding this comment

Uh oh!

tisnik commented Dec 9, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

major commented Dec 9, 2025 •

edited by coderabbitai bot

Loading

coderabbitai bot commented Dec 9, 2025 •

edited

Loading