Skip to content

Conversation

@tisnik
Copy link
Contributor

@tisnik tisnik commented Jan 13, 2026

Description

LCORE-694: new configuration option

Type of change

  • Refactor
  • New feature
  • Bug fix
  • CVE fix
  • Optimization
  • Documentation Update
  • Configuration Update
  • Bump-up service version
  • Bump-up dependent library
  • Bump-up library or tool used for development (does not change the final image)
  • CI configuration change
  • Konflux configuration change
  • Unit tests improvement
  • Integration tests improvement
  • End to end tests improvement

Tools used to create PR

  • Assisted-by: N/A
  • Generated by: N/A

Related Tickets & Documents

  • Related Issue #LCORE-694

Summary by CodeRabbit

  • New Features

    • Added skip_for_readiness_probe option to AuthenticationConfiguration to allow skipping authentication for readiness and liveness probes (default: false).
  • Documentation

    • API and configuration docs updated to include the new field and its description.
  • Tests

    • Unit tests updated to cover the new configuration field and its serialized output.

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

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 13, 2026

Warning

Rate limit exceeded

@tisnik has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 12 minutes and 0 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

📥 Commits

Reviewing files that changed from the base of the PR and between ebefc88 and 718be3d.

📒 Files selected for processing (6)
  • docs/config.html
  • docs/config.md
  • docs/openapi.json
  • src/models/config.py
  • tests/unit/models/config/test_authentication_configuration.py
  • tests/unit/models/config/test_dump_configuration.py

Walkthrough

Adds a new boolean field skip_for_readiness_probe to AuthenticationConfiguration (models, docs, OpenAPI, and tests). The field defaults to false and documents skipping authentication for readiness/liveness probes. All changes are additive; no existing behavior removed.

Changes

Cohort / File(s) Summary
Documentation
docs/config.html, docs/config.md, docs/openapi.json
Documented new boolean skip_for_readiness_probe in the AuthenticationConfiguration schema and added HTML table layout changes to accommodate the new row.
Model
src/models/config.py
Added skip_for_readiness_probe: bool to AuthenticationConfiguration with default False; updated Configuration.authentication default_factory to initialize the field.
Tests
tests/unit/models/config/test_authentication_configuration.py, tests/unit/models/config/test_dump_configuration.py
Updated and added unit tests to assert the presence and default/explicit behavior of skip_for_readiness_probe in model construction and serialized dumps.

Sequence Diagram(s)

(omitted — changes are additive config + docs/tests and do not introduce multi-component control flow)

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Suggested labels

ok-to-test

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 inconclusive)
Check name Status Explanation Resolution
Title check ❓ Inconclusive The title 'LCORE-694: new configuration option' is vague and generic, using non-descriptive language that doesn't convey what the configuration option does or its specific purpose. Consider adding a more specific descriptor (e.g., 'LCORE-694: add skip_for_readiness_probe configuration option') to clarify the change rather than using the generic 'new configuration option'.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


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.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/models/config.py (1)

1527-1531: Fix type error: default_factory requires a callable, not a class.

The pipeline failures (Pyright and mypy) indicate that passing AuthenticationConfiguration directly to default_factory is invalid. Pydantic expects a zero-argument callable that returns an instance.

🔧 Proposed fix
     authentication: AuthenticationConfiguration = Field(
-        default_factory=AuthenticationConfiguration,
+        default_factory=lambda: AuthenticationConfiguration(),
         title="Authentication configuration",
         description="Authentication configuration",
     )
🤖 Fix all issues with AI agents
In @docs/openapi.json:
- Around line 4894-4899: The schema property skip_for_readiness_probe is
misleading vs its description; rename it to skip_for_probes or
skip_for_health_probes (pick one consistently) and update its "title" and
"description" to say "Skip authentication for readiness and liveness probes" (or
similar) so name and text match, then update all /v1/config response examples to
include the new field (set explicitly to false if default) so examples match the
schema; adjust any references to skip_for_readiness_probe elsewhere in the spec
to the chosen new identifier to keep the OpenAPI spec consistent.
🧹 Nitpick comments (1)
tests/unit/models/config/test_authentication_configuration.py (1)

300-301: Consider a more descriptive docstring.

The docstring could better describe the specific behavior being tested. Per coding guidelines, docstrings should be descriptive.

📝 Suggested docstring improvement
 def test_authentication_configuration_skip_readiness_probe() -> None:
-    """Test the authentication configuration in main config."""
+    """Test skip_for_readiness_probe field in authentication configuration.
+
+    Verify that skip_for_readiness_probe=True is correctly stored
+    and accessible through Configuration.authentication.
+    """
📜 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 d649176 and d8414d2.

📒 Files selected for processing (6)
  • docs/config.html
  • docs/config.md
  • docs/openapi.json
  • src/models/config.py
  • tests/unit/models/config/test_authentication_configuration.py
  • tests/unit/models/config/test_dump_configuration.py
🧰 Additional context used
📓 Path-based instructions (5)
**/*.py

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.py: Use absolute imports for internal modules: from authentication import get_auth_dependency
Use FastAPI dependencies: from fastapi import APIRouter, HTTPException, Request, status, Depends
Use Llama Stack imports: from llama_stack_client import AsyncLlamaStackClient
Check constants.py for shared constants before defining new ones
All modules start with descriptive docstrings explaining purpose
Use logger = logging.getLogger(__name__) pattern for module logging
Type aliases defined at module level for clarity
All functions require docstrings with brief descriptions
Complete type annotations for function parameters and return types, using typing_extensions.Self for model validators
Use union types with modern syntax: str | int instead of Union[str, int]
Use Optional[Type] for optional parameters
Use snake_case with descriptive, action-oriented function names (get_, validate_, check_)
Avoid in-place parameter modification anti-patterns; return new data structures instead
Use async def for I/O operations and external API calls
Handle APIConnectionError from Llama Stack in error handling
Use logger.debug() for detailed diagnostic information
Use logger.info() for general information about program execution
Use logger.warning() for unexpected events or potential problems
Use logger.error() for serious problems that prevented function execution
All classes require descriptive docstrings explaining purpose
Use PascalCase for class names with descriptive names and standard suffixes: Configuration, Error/Exception, Resolver, Interface
Use ABC for abstract base classes with @abstractmethod decorators
Complete type annotations for all class attributes
Follow Google Python docstring conventions (https://google.github.io/styleguide/pyguide.html) with sections: Args, Returns, Raises, Attributes
Run uv run make format to auto-format code with black and ruff before completion
Run uv run make verify to run all linters (black, pyl...

Files:

  • tests/unit/models/config/test_dump_configuration.py
  • tests/unit/models/config/test_authentication_configuration.py
  • src/models/config.py
tests/**/*.py

📄 CodeRabbit inference engine (CLAUDE.md)

tests/**/*.py: Use pytest for all unit and integration tests, not unittest
Use pytest-mock for AsyncMock objects in tests
Use MOCK_AUTH = ("mock_user_id", "mock_username", False, "mock_token") pattern for authentication mocks in tests

Files:

  • tests/unit/models/config/test_dump_configuration.py
  • tests/unit/models/config/test_authentication_configuration.py
tests/unit/**/*.py

📄 CodeRabbit inference engine (CLAUDE.md)

tests/unit/**/*.py: Unit tests require 60% code coverage
Write unit tests covering new functionality before completion

Files:

  • tests/unit/models/config/test_dump_configuration.py
  • tests/unit/models/config/test_authentication_configuration.py
src/models/config.py

📄 CodeRabbit inference engine (CLAUDE.md)

src/models/config.py: All config uses Pydantic models extending ConfigurationBase
Configuration base class sets extra="forbid" to reject unknown fields
Use type hints: Optional[FilePath], PositiveInt, SecretStr for configuration fields
Extend ConfigurationBase for configuration Pydantic 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
Extend BaseModel for data Pydantic models
Use @model_validator and @field_validator for Pydantic model validation

Files:

  • src/models/config.py
🧠 Learnings (2)
📚 Learning: 2026-01-11T16:30:41.784Z
Learnt from: CR
Repo: lightspeed-core/lightspeed-stack PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-11T16:30:41.784Z
Learning: Applies to src/authentication/k8s.py : Pyright type checking excludes `src/authentication/k8s.py`

Applied to files:

  • tests/unit/models/config/test_dump_configuration.py
  • src/models/config.py
📚 Learning: 2026-01-12T10:58:40.230Z
Learnt from: blublinsky
Repo: lightspeed-core/lightspeed-stack PR: 972
File: src/models/config.py:459-513
Timestamp: 2026-01-12T10:58:40.230Z
Learning: In lightspeed-core/lightspeed-stack, for Python files under src/models, when a user claims a fix is done but the issue persists, verify the current code state before accepting the fix. Steps: review the diff, fetch the latest changes, run relevant tests, reproduce the issue, search the codebase for lingering references to the original problem, confirm the fix is applied and not undone by subsequent commits, and validate with local checks to ensure the issue is resolved.

Applied to files:

  • src/models/config.py
🧬 Code graph analysis (1)
tests/unit/models/config/test_authentication_configuration.py (1)
src/models/config.py (2)
  • Configuration (1482-1602)
  • AuthenticationConfiguration (940-1056)
🪛 GitHub Actions: Pyright
src/models/config.py

[error] 1528-1528: Step: uv run pyright src. Pyright error in config.py: Argument of type 'type[AuthenticationConfiguration]' cannot be assigned to parameter 'default_factory' of type '(() -> _T@Field) | ((dict[str, Any]) -> _T@Field)' in function 'Field'; Type 'type[AuthenticationConfiguration]' is not assignable to type '(dict[str, Any]) -> _T@Field'.

🪛 GitHub Actions: Type checks
src/models/config.py

[error] 1528-1528: Argument "default_factory" to "Field" has incompatible type "type[AuthenticationConfiguration]"; expected "Callable[[], Never] | Callable[[dict[str, Any]], Never]" (mypy) while running: uv run mypy --explicit-package-bases --disallow-untyped-calls --disallow-untyped-defs --disallow-incomplete-defs --ignore-missing-imports --disable-error-code attr-defined src/

⏰ 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). (4)
  • GitHub Check: build-pr
  • GitHub Check: Konflux kflux-prd-rh02 / lightspeed-stack-on-pull-request
  • GitHub Check: E2E: server mode / ci
  • GitHub Check: E2E: library mode / ci
🔇 Additional comments (7)
src/models/config.py (1)

945-951: LGTM! New configuration field is well-defined.

The skip_for_readiness_probe field follows the established pattern with proper use of Field(), including a default value, title, and description. The comment on line 946 provides helpful context linking to the JIRA ticket.

docs/config.md (1)

75-75: LGTM! Documentation correctly reflects the new configuration field.

The field description and type match the implementation in config.py.

docs/config.html (1)

264-291: LGTM! HTML documentation correctly updated.

The new skip_for_readiness_probe field is properly added to the AuthenticationConfiguration table with correct type and description. The column group styling and row class alternation are appropriately adjusted.

tests/unit/models/config/test_dump_configuration.py (2)

152-161: LGTM!

The expected authentication section correctly includes the new skip_for_readiness_probe field with default value False. This change is consistent with the model definition and properly placed in the expected output structure.


462-471: Consistent update across test functions.

The expected output correctly reflects the new skip_for_readiness_probe field. All occurrences in this file (lines 155, 465, 681, 877, 1062) are consistent.

tests/unit/models/config/test_authentication_configuration.py (2)

29-46: LGTM!

The test correctly verifies the new skip_for_readiness_probe field with explicit initialization to False and proper assertion. Good addition of coverage for the default behavior.


300-327: LGTM! Good coverage for the new configuration option.

The new test function properly verifies that skip_for_readiness_probe=True is correctly stored and retrievable through the Configuration.authentication path. Combined with the existing test (line 42) that verifies False, this provides good coverage for the new field.

Comment on lines 4894 to 4899
"skip_for_readiness_probe": {
"type": "boolean",
"title": "Skip authentication for probes",
"description": "Skip authentication for readiness and liveness probes",
"default": false
},
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Update spec examples + consider renaming for clarity (“readiness” vs “readiness & liveness”).

The new skip_for_readiness_probe schema property is fine, but:

  • The name suggests readiness only while the description says readiness and liveness; consider renaming to something like skip_for_probes / skip_for_health_probes (still easy since this is newly introduced).
  • This file’s /v1/config response examples appear to omit the new field (even if it’s false by default), which makes the OpenAPI examples drift from the schema.
🤖 Prompt for AI Agents
In @docs/openapi.json around lines 4894 - 4899, The schema property
skip_for_readiness_probe is misleading vs its description; rename it to
skip_for_probes or skip_for_health_probes (pick one consistently) and update its
"title" and "description" to say "Skip authentication for readiness and liveness
probes" (or similar) so name and text match, then update all /v1/config response
examples to include the new field (set explicitly to false if default) so
examples match the schema; adjust any references to skip_for_readiness_probe
elsewhere in the spec to the chosen new identifier to keep the OpenAPI spec
consistent.

@tisnik tisnik force-pushed the lcore-694-new-configuration-option branch from d8414d2 to fb454ad Compare January 13, 2026 10:35
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In @src/models/config.py:
- Around line 946-951: The inline comment above the skip_for_readiness_probe
Field incorrectly uses "authorization"; update the comment to say
"authentication" so it matches the AuthenticationConfiguration context and the
Field description, e.g., change the comment text "Config option to skip
authorization for readiness and liveness probe" to "Config option to skip
authentication for readiness and liveness probes" and ensure the identifier
skip_for_readiness_probe remains unchanged.
📜 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 d8414d2 and fb454ad.

📒 Files selected for processing (4)
  • docs/config.html
  • docs/config.md
  • docs/openapi.json
  • src/models/config.py
🚧 Files skipped from review as they are similar to previous changes (3)
  • docs/config.md
  • docs/config.html
  • docs/openapi.json
🧰 Additional context used
📓 Path-based instructions (3)
**/*.py

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.py: Use absolute imports for internal modules: from authentication import get_auth_dependency
Use FastAPI dependencies: from fastapi import APIRouter, HTTPException, Request, status, Depends
Use Llama Stack imports: from llama_stack_client import AsyncLlamaStackClient
Check constants.py for shared constants before defining new ones
All modules start with descriptive docstrings explaining purpose
Use logger = logging.getLogger(__name__) pattern for module logging
Type aliases defined at module level for clarity
All functions require docstrings with brief descriptions
Complete type annotations for function parameters and return types, using typing_extensions.Self for model validators
Use union types with modern syntax: str | int instead of Union[str, int]
Use Optional[Type] for optional parameters
Use snake_case with descriptive, action-oriented function names (get_, validate_, check_)
Avoid in-place parameter modification anti-patterns; return new data structures instead
Use async def for I/O operations and external API calls
Handle APIConnectionError from Llama Stack in error handling
Use logger.debug() for detailed diagnostic information
Use logger.info() for general information about program execution
Use logger.warning() for unexpected events or potential problems
Use logger.error() for serious problems that prevented function execution
All classes require descriptive docstrings explaining purpose
Use PascalCase for class names with descriptive names and standard suffixes: Configuration, Error/Exception, Resolver, Interface
Use ABC for abstract base classes with @abstractmethod decorators
Complete type annotations for all class attributes
Follow Google Python docstring conventions (https://google.github.io/styleguide/pyguide.html) with sections: Args, Returns, Raises, Attributes
Run uv run make format to auto-format code with black and ruff before completion
Run uv run make verify to run all linters (black, pyl...

Files:

  • src/models/config.py
src/models/config.py

📄 CodeRabbit inference engine (CLAUDE.md)

src/models/config.py: All config uses Pydantic models extending ConfigurationBase
Configuration base class sets extra="forbid" to reject unknown fields
Use type hints: Optional[FilePath], PositiveInt, SecretStr for configuration fields
Extend ConfigurationBase for configuration Pydantic 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
Extend BaseModel for data Pydantic models
Use @model_validator and @field_validator for Pydantic model validation

Files:

  • src/models/config.py
🧠 Learnings (3)
📚 Learning: 2026-01-11T16:30:41.784Z
Learnt from: CR
Repo: lightspeed-core/lightspeed-stack PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-11T16:30:41.784Z
Learning: Applies to src/models/config.py : All config uses Pydantic models extending `ConfigurationBase`

Applied to files:

  • src/models/config.py
📚 Learning: 2026-01-11T16:30:41.784Z
Learnt from: CR
Repo: lightspeed-core/lightspeed-stack PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-11T16:30:41.784Z
Learning: Applies to src/authentication/k8s.py : Pyright type checking excludes `src/authentication/k8s.py`

Applied to files:

  • src/models/config.py
📚 Learning: 2026-01-12T10:58:40.230Z
Learnt from: blublinsky
Repo: lightspeed-core/lightspeed-stack PR: 972
File: src/models/config.py:459-513
Timestamp: 2026-01-12T10:58:40.230Z
Learning: In lightspeed-core/lightspeed-stack, for Python files under src/models, when a user claims a fix is done but the issue persists, verify the current code state before accepting the fix. Steps: review the diff, fetch the latest changes, run relevant tests, reproduce the issue, search the codebase for lingering references to the original problem, confirm the fix is applied and not undone by subsequent commits, and validate with local checks to ensure the issue is resolved.

Applied to files:

  • src/models/config.py
⏰ 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: Pylinter
  • GitHub Check: build-pr
  • GitHub Check: Konflux kflux-prd-rh02 / lightspeed-stack-on-pull-request
  • GitHub Check: E2E: library mode / ci
  • GitHub Check: E2E: server mode / ci
🔇 Additional comments (1)
src/models/config.py (1)

1527-1533: LGTM!

The explicit initialization of skip_for_readiness_probe=False in the default factory is consistent with the existing pattern used for other configuration fields in this class (e.g., DatabaseConfiguration, InferenceConfiguration, QuotaHandlersConfiguration).

Comment on lines 946 to 951
# LCORE-694: Config option to skip authorization for readiness and liveness probe
skip_for_readiness_probe: bool = Field(
False,
title="Skip authentication for probes",
description="Skip authentication for readiness and liveness probes",
)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Comment typo: "authorization" should be "authentication".

The comment references skipping "authorization" but this field controls skipping authentication for probes. Since this is in AuthenticationConfiguration and the description correctly says "Skip authentication for readiness and liveness probes", the comment should match.

Suggested fix
-    # LCORE-694: Config option to skip authorization for readiness and liveness probe
+    # LCORE-694: Config option to skip authentication for readiness and liveness probe
🤖 Prompt for AI Agents
In @src/models/config.py around lines 946 - 951, The inline comment above the
skip_for_readiness_probe Field incorrectly uses "authorization"; update the
comment to say "authentication" so it matches the AuthenticationConfiguration
context and the Field description, e.g., change the comment text "Config option
to skip authorization for readiness and liveness probe" to "Config option to
skip authentication for readiness and liveness probes" and ensure the identifier
skip_for_readiness_probe remains unchanged.

@tisnik tisnik force-pushed the lcore-694-new-configuration-option branch 2 times, most recently from ebefc88 to 5e9bf43 Compare January 13, 2026 10:45
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In @docs/config.md:
- Line 75: The docs currently use mixed terms; update the field description for
skip_for_health_probes to use a single term consistently (choose
"authentication" or "authorization") — e.g., change all occurrences to
"authentication" so the description reads like "Skip authentication for
readiness and liveness probes"; apply this same wording in docs/config.md,
docs/config.html and the openapi schema (docs/openapi.json) so the
schema.description for skip_for_health_probes matches the human docs and correct
the field name in any snippet that still shows skip_for_readiness_probe.
🧹 Nitpick comments (1)
src/models/config.py (1)

1527-1530: Consider simplifying the default_factory (optional).

The explicit skip_for_readiness_probe=False is redundant since the field already defaults to False in AuthenticationConfiguration. The simpler form would work identically:

default_factory=AuthenticationConfiguration

That said, explicit initialization aligns with patterns elsewhere in this file (e.g., DatabaseConfiguration, InferenceConfiguration). If the intent is documentation/clarity, this is acceptable.

📜 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 fb454ad and ebefc88.

📒 Files selected for processing (4)
  • docs/config.html
  • docs/config.md
  • docs/openapi.json
  • src/models/config.py
🚧 Files skipped from review as they are similar to previous changes (2)
  • docs/openapi.json
  • docs/config.html
🧰 Additional context used
📓 Path-based instructions (3)
**/*.py

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.py: Use absolute imports for internal modules: from authentication import get_auth_dependency
Use FastAPI dependencies: from fastapi import APIRouter, HTTPException, Request, status, Depends
Use Llama Stack imports: from llama_stack_client import AsyncLlamaStackClient
Check constants.py for shared constants before defining new ones
All modules start with descriptive docstrings explaining purpose
Use logger = logging.getLogger(__name__) pattern for module logging
Type aliases defined at module level for clarity
All functions require docstrings with brief descriptions
Complete type annotations for function parameters and return types, using typing_extensions.Self for model validators
Use union types with modern syntax: str | int instead of Union[str, int]
Use Optional[Type] for optional parameters
Use snake_case with descriptive, action-oriented function names (get_, validate_, check_)
Avoid in-place parameter modification anti-patterns; return new data structures instead
Use async def for I/O operations and external API calls
Handle APIConnectionError from Llama Stack in error handling
Use logger.debug() for detailed diagnostic information
Use logger.info() for general information about program execution
Use logger.warning() for unexpected events or potential problems
Use logger.error() for serious problems that prevented function execution
All classes require descriptive docstrings explaining purpose
Use PascalCase for class names with descriptive names and standard suffixes: Configuration, Error/Exception, Resolver, Interface
Use ABC for abstract base classes with @abstractmethod decorators
Complete type annotations for all class attributes
Follow Google Python docstring conventions (https://google.github.io/styleguide/pyguide.html) with sections: Args, Returns, Raises, Attributes
Run uv run make format to auto-format code with black and ruff before completion
Run uv run make verify to run all linters (black, pyl...

Files:

  • src/models/config.py
src/models/config.py

📄 CodeRabbit inference engine (CLAUDE.md)

src/models/config.py: All config uses Pydantic models extending ConfigurationBase
Configuration base class sets extra="forbid" to reject unknown fields
Use type hints: Optional[FilePath], PositiveInt, SecretStr for configuration fields
Extend ConfigurationBase for configuration Pydantic 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
Extend BaseModel for data Pydantic models
Use @model_validator and @field_validator for Pydantic model validation

Files:

  • src/models/config.py
🧠 Learnings (2)
📚 Learning: 2026-01-11T16:30:41.784Z
Learnt from: CR
Repo: lightspeed-core/lightspeed-stack PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-11T16:30:41.784Z
Learning: Applies to src/authentication/k8s.py : Pyright type checking excludes `src/authentication/k8s.py`

Applied to files:

  • src/models/config.py
📚 Learning: 2026-01-12T10:58:40.230Z
Learnt from: blublinsky
Repo: lightspeed-core/lightspeed-stack PR: 972
File: src/models/config.py:459-513
Timestamp: 2026-01-12T10:58:40.230Z
Learning: In lightspeed-core/lightspeed-stack, for Python files under src/models, when a user claims a fix is done but the issue persists, verify the current code state before accepting the fix. Steps: review the diff, fetch the latest changes, run relevant tests, reproduce the issue, search the codebase for lingering references to the original problem, confirm the fix is applied and not undone by subsequent commits, and validate with local checks to ensure the issue is resolved.

Applied to files:

  • src/models/config.py
⏰ 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). (4)
  • GitHub Check: Konflux kflux-prd-rh02 / lightspeed-stack-on-pull-request
  • GitHub Check: build-pr
  • GitHub Check: E2E: library mode / ci
  • GitHub Check: E2E: server mode / ci
🔇 Additional comments (1)
src/models/config.py (1)

945-951: LGTM!

The new field is well-structured:

  • Correct bool type with safe default (False)
  • Proper Field() metadata for documentation/schema generation
  • Ticket reference comment for traceability

This follows the coding guidelines for Pydantic configuration models.

@tisnik tisnik force-pushed the lcore-694-new-configuration-option branch from 5e9bf43 to 718be3d Compare January 13, 2026 10:48
@tisnik tisnik merged commit 08113e2 into lightspeed-core:main Jan 13, 2026
19 of 23 checks passed
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