LCORE-1826: Expose OpenTelemetry configuration in v1/config endpoint - #2273
LCORE-1826: Expose OpenTelemetry configuration in v1/config endpoint#2273anik120 wants to merge 1 commit into
Conversation
WalkthroughChangesObservability configuration
Lightspeed Core Stack 2.0 strategy
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Environment
participant ObservabilityConfiguration
participant Configuration
participant ConfigEndpoint
Environment->>ObservabilityConfiguration: provide OTEL_* variables
ObservabilityConfiguration->>Configuration: create observability.otel
Configuration->>ConfigEndpoint: expose configuration
ConfigEndpoint-->>Configuration: return observability.otel
Possibly related PRs
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error)
✅ Passed checks (6 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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.
Inline comments:
In `@src/models/config.py`:
- Around line 2840-2863: Update ObservabilityConfiguration.from_environment to
prevent OTEL header variables from being exposed through
ConfigurationResponse.configuration: collect only a reviewed non-sensitive
OTEL_* allowlist or explicitly redact OTEL_EXPORTER_OTLP_HEADERS,
OTEL_EXPORTER_OTLP_TRACES_HEADERS, OTEL_EXPORTER_OTLP_METRICS_HEADERS, and
OTEL_EXPORTER_OTLP_LOGS_HEADERS before constructing the model, while preserving
safe OpenTelemetry configuration values.
In `@tests/integration/endpoints/test_config_integration.py`:
- Around line 125-167: Update
test_config_endpoint_observability_collects_otel_vars to set the OTEL
environment variables before rebuilding the app/config fixture, then invoke
config_endpoint_handler and assert the values through
response.configuration.observability.otel. Remove the direct
ObservabilityConfiguration.from_environment() assertions so the test validates
endpoint behavior and catches configuration default-factory regressions.
In `@tests/unit/models/config/test_observability_configuration.py`:
- Around line 16-17: Update the affected test docstrings in the observability
configuration tests to include a Parameters: section documenting each function
argument: monkeypatch, otel_dict, and expected_count. Apply this consistently to
all referenced tests, using the parameter names and their purposes without
changing test behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: b75e3160-73fd-45a1-a556-25bfeae797cf
📒 Files selected for processing (4)
src/models/api/responses/successful/configuration.pysrc/models/config.pytests/integration/endpoints/test_config_integration.pytests/unit/models/config/test_observability_configuration.py
📜 Review details
⏰ Context from checks skipped due to timeout. (13)
- GitHub Check: bandit
- GitHub Check: Pylinter
- GitHub Check: integration_tests (3.12)
- GitHub Check: integration_tests (3.13)
- GitHub Check: build-pr
- GitHub Check: E2E: library mode / ci / group 1
- GitHub Check: E2E: server mode / ci / group 1
- GitHub Check: E2E: server mode / ci / group 2
- GitHub Check: E2E: library mode / ci / group 2
- GitHub Check: E2E: library mode / ci / group 3
- GitHub Check: E2E: server mode / ci / group 3
- GitHub Check: E2E Tests for Lightspeed Evaluation job
- GitHub Check: Konflux kflux-prd-rh02 / lightspeed-stack-0-7-on-pull-request
⚠️ CI failures not shown inline (2)
GitHub Actions: OpenAPI (Spectral) / spectral: LCORE-1826: Expose OpenTelemetry configuration in v1/config endpoint
Conclusion: failure
##[group]Run set -euo pipefail
�[36;1mset -euo pipefail�[0m
�[36;1muv run python scripts/generate_openapi_schema.py /tmp/openapi-generated.json�[0m
�[36;1mif ! diff -u docs/devel_doc/openapi.json /tmp/openapi-generated.json; then�[0m
�[36;1m echo "::error::docs/devel_doc/openapi.json is out of date. Regenerate with: uv run scripts/generate_openapi_schema.py docs/devel_doc/openapi.json"�[0m
GitHub Actions: OpenAPI (Spectral) / 0_spectral.txt: LCORE-1826: Expose OpenTelemetry configuration in v1/config endpoint
Conclusion: failure
##[group]Run set -euo pipefail
�[36;1mset -euo pipefail�[0m
�[36;1muv run python scripts/generate_openapi_schema.py /tmp/openapi-generated.json�[0m
�[36;1mif ! diff -u docs/devel_doc/openapi.json /tmp/openapi-generated.json; then�[0m
�[36;1m echo "::error::docs/devel_doc/openapi.json is out of date. Regenerate with: uv run scripts/generate_openapi_schema.py docs/devel_doc/openapi.json"�[0m
🧰 Additional context used
📓 Path-based instructions (5)
**/*
📄 CodeRabbit inference engine (Custom checks)
**/*: Flag meaningful O(n^2)+ algorithms on non-trivial inputs, including handlers and Kubernetes list operations.
Flag N+1 patterns that list items and then query once per item, including Kubernetes API and database access.
Flag expensive work inside loops, including API calls, JSON parsing, and regex compilation.
Flag unbounded growth in caches, watchers, or buffers when eviction or limits are missing.
Flag missing pagination or limits on list operations and API endpoints.
Flag secrets or tokens logged in plaintext or hardcoded in source.
Flag API endpoints missing authentication or authorization.
Flag injection vulnerabilities, including SQL injection, command injection, and path traversal.
Flag sensitive data leaked in API responses, WebSocket messages, or logs.
Flag Kubernetes Secrets and Red Hat secrets missing OwnerReferences.
Files:
src/models/api/responses/successful/configuration.pytests/unit/models/config/test_observability_configuration.pysrc/models/config.pytests/integration/endpoints/test_config_integration.py
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/*.py: Use absolute imports for internal Python modules.
All modules must begin with descriptive docstrings explaining their purpose.
Uselogger = get_logger(__name__)fromlog.pyfor module logging.
Use complete type annotations for function parameters and return types.
Use modern union syntax such asstr | int; useOptional[Type]for optional values.
Usetyping_extensions.Selffor model validators.
Functions must use descriptive, action-oriented snake_case names such asget_,validate_, andcheck_.
Avoid modifying mutable parameters in place; return a new data structure instead.
Useasync deffor I/O operations and external API calls.
HandleAPIConnectionErrorfrom Llama Stack.
All classes must have descriptive docstrings and complete type annotations for class attributes; use specific types instead ofAny.
Use PascalCase for classes and descriptive standard suffixes such asConfiguration,Error/Exception,Resolver, andInterface.
Use ABC and@abstractmethodfor abstract interfaces.
Follow Google Python docstring conventions; document all modules, classes, and functions, includingParameters,Returns,Raises, andAttributessections as applicable.
Never commit secrets or keys; use environment variables for sensitive data.
Files:
src/models/api/responses/successful/configuration.pytests/unit/models/config/test_observability_configuration.pysrc/models/config.pytests/integration/endpoints/test_config_integration.py
src/models/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
src/models/**/*.py: ExtendConfigurationBasefor configuration models andBaseModelfor data models.
Use@model_validatorand@field_validatorfor Pydantic model validation.
Files:
src/models/api/responses/successful/configuration.pysrc/models/config.py
tests/unit/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
tests/unit/**/*.py: Use pytest for all unit tests; do not use unittest.
Useconftest.pyfor shared fixtures,pytest-mockforAsyncMockobjects, andpytest.mark.asynciofor asynchronous tests.
Files:
tests/unit/models/config/test_observability_configuration.py
tests/integration/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Use pytest for all integration tests; do not use unittest.
Files:
tests/integration/endpoints/test_config_integration.py
🧠 Learnings (5)
📚 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/api/responses/successful/configuration.pysrc/models/config.py
📚 Learning: 2026-02-25T07:46:33.545Z
Learnt from: asimurka
Repo: lightspeed-core/lightspeed-stack PR: 1211
File: src/models/responses.py:8-16
Timestamp: 2026-02-25T07:46:33.545Z
Learning: In the Python codebase, requests.py should use OpenAIResponseInputTool as Tool while responses.py uses OpenAIResponseTool as Tool. This difference is intentional due to differing schemas for input vs output tools in llama-stack-api. Apply this distinction consistently to other models under src/models (e.g., ensure request-related tools use the InputTool variant and response-related tools use the ResponseTool variant). If adding new tools, choose the corresponding InputTool or Tool class based on whether the tool represents input or output, and document the rationale in code comments.
Applied to files:
src/models/api/responses/successful/configuration.pysrc/models/config.py
📚 Learning: 2026-06-24T13:45:37.249Z
Learnt from: Jdubrick
Repo: lightspeed-core/lightspeed-stack PR: 1971
File: src/utils/markdown_repair.py:31-36
Timestamp: 2026-06-24T13:45:37.249Z
Learning: In the lightspeed-stack repository, docstrings must use the section header name "Parameters:" (not "Args:") for function arguments, even if the project references Google Python docstring conventions. Ensure docstrings follow the project’s established "Parameters:" header format for any documented function parameters.
Applied to files:
src/models/api/responses/successful/configuration.pytests/unit/models/config/test_observability_configuration.pysrc/models/config.pytests/integration/endpoints/test_config_integration.py
📚 Learning: 2026-07-06T15:26:18.398Z
Learnt from: Jdubrick
Repo: lightspeed-core/lightspeed-stack PR: 2071
File: src/models/config.py:2416-2422
Timestamp: 2026-07-06T15:26:18.398Z
Learning: In this repo’s Python code under src/**, don’t treat differences in string concatenation style as a style inconsistency when Black has effectively forced (or made clearer) use of explicit `+` string concatenation in multi-line logger/string expressions. If adjacent-literal implicit concatenation is avoided/changed specifically to accommodate Black’s formatting in these call sites, accept the `+` usage and don’t recommend converting it solely for consistency with nearby blocks that use implicit concatenation.
Applied to files:
src/models/api/responses/successful/configuration.pysrc/models/config.py
📚 Learning: 2026-07-17T19:25:05.325Z
Learnt from: Jdubrick
Repo: lightspeed-core/lightspeed-stack PR: 2166
File: src/utils/saved_prompts.py:129-157
Timestamp: 2026-07-17T19:25:05.325Z
Learning: For any endpoint that handles saved prompts and calls `src/utils/saved_prompts.py::create_saved_prompt`, treat the endpoint as the validation boundary. Before calling `create_saved_prompt`, validate the incoming saved-prompt name and content, specifically using `validate_saved_prompt_name` and then persist (store) the normalized value it returns. Do not call `create_saved_prompt` with unvalidated/raw name/content.
Applied to files:
src/models/api/responses/successful/configuration.pysrc/models/config.py
🔇 Additional comments (1)
src/models/api/responses/successful/configuration.py (1)
90-97: LGTM!
| async def test_config_endpoint_observability_collects_otel_vars( | ||
| current_config: AppConfig, # pylint: disable=unused-argument | ||
| test_request: Request, # pylint: disable=unused-argument | ||
| test_auth: AuthTuple, # pylint: disable=unused-argument | ||
| monkeypatch: pytest.MonkeyPatch, | ||
| ) -> None: | ||
| """Test that observability config collects OTEL_* environment variables. | ||
|
|
||
| This integration test verifies: | ||
| - OTEL_* environment variables are collected into observability.otel | ||
| - Non-OTEL variables are not included | ||
| - Environment variables set at runtime are reflected in config | ||
|
|
||
| Parameters: | ||
| ---------- | ||
| current_config (AppConfig): Loads root configuration | ||
| test_request (Request): FastAPI request | ||
| test_auth (AuthTuple): noop authentication tuple | ||
| monkeypatch (pytest.MonkeyPatch): Fixture to modify environment variables | ||
| """ | ||
| # pylint: disable=import-outside-toplevel | ||
| from models.config import ObservabilityConfiguration | ||
|
|
||
| # Set OTEL environment variables | ||
| monkeypatch.setenv("OTEL_SDK_DISABLED", "true") | ||
| monkeypatch.setenv("OTEL_SERVICE_NAME", "test-service") | ||
| monkeypatch.setenv("OTEL_EXPORTER_OTLP_ENDPOINT", "http://localhost:4317") | ||
|
|
||
| # Reload configuration to pick up new env vars | ||
| updated_observability = ObservabilityConfiguration.from_environment() | ||
|
|
||
| # Verify OTEL vars are present in the environment | ||
| # (Note: The config is loaded at startup, so these may not be in the response | ||
| # unless we reload the entire config. This test verifies the mechanism works.) | ||
| assert "OTEL_SDK_DISABLED" in os.environ | ||
| assert "OTEL_SERVICE_NAME" in os.environ | ||
| assert "OTEL_EXPORTER_OTLP_ENDPOINT" in os.environ | ||
|
|
||
| # Verify that when we call from_environment(), it picks up the vars | ||
| assert "OTEL_SDK_DISABLED" in updated_observability.otel | ||
| assert updated_observability.otel["OTEL_SDK_DISABLED"] == "true" | ||
| assert "OTEL_SERVICE_NAME" in updated_observability.otel | ||
| assert updated_observability.otel["OTEL_SERVICE_NAME"] == "test-service" |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Assert the mutated values through the endpoint.
This test never invokes config_endpoint_handler after setting the variables; it only re-tests ObservabilityConfiguration.from_environment(). Set the environment before rebuilding the app/config fixture, call the endpoint, and assert the OTEL values in response.configuration.observability.otel. Otherwise removal of the Configuration default factory can still pass this integration test.
🤖 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 `@tests/integration/endpoints/test_config_integration.py` around lines 125 -
167, Update test_config_endpoint_observability_collects_otel_vars to set the
OTEL environment variables before rebuilding the app/config fixture, then invoke
config_endpoint_handler and assert the values through
response.configuration.observability.otel. Remove the direct
ObservabilityConfiguration.from_environment() assertions so the test validates
endpoint behavior and catches configuration default-factory regressions.
| def test_from_environment_no_otel_vars(monkeypatch: pytest.MonkeyPatch) -> None: | ||
| """Test from_environment with no OTEL_* environment variables.""" |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Document test parameters.
Add Parameters: sections for monkeypatch, otel_dict, and expected_count in these test docstrings.
Based on learnings, this repository uses the Parameters: header for function arguments.
Also applies to: 27-28, 48-51, 100-101
🤖 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 `@tests/unit/models/config/test_observability_configuration.py` around lines 16
- 17, Update the affected test docstrings in the observability configuration
tests to include a Parameters: section documenting each function argument:
monkeypatch, otel_dict, and expected_count. Apply this consistently to all
referenced tests, using the parameter names and their purposes without changing
test behavior.
Sources: Coding guidelines, Learnings
Add observability configuration to the v1/config endpoint, exposing all OTEL_* environment variables to provide visibility into the active OpenTelemetry tracing setup.
There was a problem hiding this comment.
Actionable comments posted: 9
🤖 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.
Inline comments:
In `@docs/LCORE-Strategic-Path-Forward.md`:
- Around line 42-59: Update the fenced code blocks in
docs/LCORE-Strategic-Path-Forward.md, including the diagrams around the
“Lightspeed Core Stack 2.0” section and the other referenced blocks, by adding
an appropriate language identifier such as text and blank lines before and after
each fence. Preserve all diagram content and formatting.
- Around line 96-107: Update the Table of Contents links for “The Problem” and
“The Opportunity” to use the current heading anchors, replacing the stale
problem slug with `#the-problem-lcores-failed-abstraction` and matching the
opportunity link to the slug generated by the document’s current opportunity
heading. Leave the remaining entries unchanged.
- Around line 61-69: Update the timeline and all referenced date sections to use
internally consistent calendar dates: correct the weekday labels for July 22,
July 26, and August 5, replace any relative “today” wording with an absolute
date, and align the 10-week plan starting August 5, 2026 with its approximately
October 14 completion rather than mid-September. Ensure the Gemini deadline
statement and the sections covering the additional referenced ranges reflect the
same schedule.
- Around line 201-216: Clarify the extraction gate’s coupling metric near the
“Rediscovering the Original Insight” analysis and its related checkpoint
sections. Define the reproducible counting method, categorize which dependencies
count as OpenShift coupling, state the denominator used for the percentage, and
ensure the “94 references,” “95% product-agnostic,” and “>20% coupling”
threshold use the same metric.
- Around line 35-37: Reconcile the build-from-scratch estimate throughout the
document, including the headline, comparison table, and 120/150-week scenarios,
by selecting one consistent baseline. Recalculate the time-advantage ratio and
all “10x faster” or related comparison claims from that baseline, and update the
decision analysis accordingly.
- Around line 174-184: Revise the “The Forcing Function: OGX Decommissioning”
section so it distinguishes confirmed milestones from the unconfirmed
decommissioning date. In the “If we stay on OGX” and “LCORE’s foundation”
statements, cite only the confirmed July announcement and explicitly frame the
strategic decision as contingent on the decommissioning timeline remaining
unconfirmed.
- Around line 373-417: The minimax analysis must use consistent probability and
cost units. Update the “Interpretation” section to retain the documented 10%
Build worst-case probability, or explicitly label 30% as a separate assumption
supported elsewhere; also either state the loaded dollar cost per engineering
week before using $16K/$600K or replace those dollar amounts with the
established 4 and 150+ eng-week values. Keep the scenario matrix, minimax
calculation, and conclusion aligned.
In `@tests/integration/endpoints/test_config_integration.py`:
- Around line 133-167: Extend the test around
ObservabilityConfiguration.from_environment() to assert
OTEL_EXPORTER_OTLP_ENDPOINT is present in updated_observability.otel with its
configured value, then set a non-OTEL environment variable and assert it is
excluded from updated_observability.otel.
In `@tests/unit/models/config/test_dump_configuration.py`:
- Around line 47-50: Isolate OTEL environment state in the dump tests by adding
a fixture that clears all OTEL_* variables, or by supplying an explicit empty
ObservabilityConfiguration when constructing Configuration. Ensure assertions
against _DEFAULT_OBSERVABILITY_DUMP remain deterministic regardless of the test
runner environment.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: f547b912-3718-4665-8e8d-c91579aa2798
📒 Files selected for processing (7)
docs/LCORE-Strategic-Path-Forward.mddocs/devel_doc/openapi.jsonsrc/models/api/responses/successful/configuration.pysrc/models/config.pytests/integration/endpoints/test_config_integration.pytests/unit/models/config/test_dump_configuration.pytests/unit/models/config/test_observability_configuration.py
📜 Review details
⏰ Context from checks skipped due to timeout. (13)
- GitHub Check: Pylinter
- GitHub Check: unit_tests (3.12)
- GitHub Check: integration_tests (3.12)
- GitHub Check: integration_tests (3.13)
- GitHub Check: build-pr
- GitHub Check: Konflux kflux-prd-rh02 / lightspeed-stack-0-7-on-pull-request
- GitHub Check: E2E Tests for Lightspeed Evaluation job
- GitHub Check: E2E: server mode / ci / group 1
- GitHub Check: E2E: library mode / ci / group 2
- GitHub Check: E2E: server mode / ci / group 2
- GitHub Check: E2E: library mode / ci / group 1
- GitHub Check: E2E: server mode / ci / group 3
- GitHub Check: E2E: library mode / ci / group 3
🧰 Additional context used
📓 Path-based instructions (5)
**/*
📄 CodeRabbit inference engine (Custom checks)
**/*: Flag meaningful O(n^2)+ algorithms on non-trivial inputs, including handlers and Kubernetes list operations.
Flag N+1 patterns that list items and then query once per item, including Kubernetes API and database access.
Flag expensive work inside loops, including API calls, JSON parsing, and regex compilation.
Flag unbounded growth in caches, watchers, or buffers when eviction or limits are missing.
Flag missing pagination or limits on list operations and API endpoints.
Flag secrets or tokens logged in plaintext or hardcoded in source.
Flag API endpoints missing authentication or authorization.
Flag injection vulnerabilities, including SQL injection, command injection, and path traversal.
Flag sensitive data leaked in API responses, WebSocket messages, or logs.
Flag Kubernetes Secrets and Red Hat secrets missing OwnerReferences.
Files:
src/models/api/responses/successful/configuration.pysrc/models/config.pydocs/devel_doc/openapi.jsontests/integration/endpoints/test_config_integration.pytests/unit/models/config/test_observability_configuration.pydocs/LCORE-Strategic-Path-Forward.mdtests/unit/models/config/test_dump_configuration.py
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/*.py: Use absolute imports for internal Python modules.
All modules must begin with descriptive docstrings explaining their purpose.
Uselogger = get_logger(__name__)fromlog.pyfor module logging.
Use complete type annotations for function parameters and return types.
Use modern union syntax such asstr | int; useOptional[Type]for optional values.
Usetyping_extensions.Selffor model validators.
Functions must use descriptive, action-oriented snake_case names such asget_,validate_, andcheck_.
Avoid modifying mutable parameters in place; return a new data structure instead.
Useasync deffor I/O operations and external API calls.
HandleAPIConnectionErrorfrom Llama Stack.
All classes must have descriptive docstrings and complete type annotations for class attributes; use specific types instead ofAny.
Use PascalCase for classes and descriptive standard suffixes such asConfiguration,Error/Exception,Resolver, andInterface.
Use ABC and@abstractmethodfor abstract interfaces.
Follow Google Python docstring conventions; document all modules, classes, and functions, includingParameters,Returns,Raises, andAttributessections as applicable.
Never commit secrets or keys; use environment variables for sensitive data.
Files:
src/models/api/responses/successful/configuration.pysrc/models/config.pytests/integration/endpoints/test_config_integration.pytests/unit/models/config/test_observability_configuration.pytests/unit/models/config/test_dump_configuration.py
src/models/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
src/models/**/*.py: ExtendConfigurationBasefor configuration models andBaseModelfor data models.
Use@model_validatorand@field_validatorfor Pydantic model validation.
Files:
src/models/api/responses/successful/configuration.pysrc/models/config.py
tests/integration/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Use pytest for all integration tests; do not use unittest.
Files:
tests/integration/endpoints/test_config_integration.py
tests/unit/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
tests/unit/**/*.py: Use pytest for all unit tests; do not use unittest.
Useconftest.pyfor shared fixtures,pytest-mockforAsyncMockobjects, andpytest.mark.asynciofor asynchronous tests.
Files:
tests/unit/models/config/test_observability_configuration.pytests/unit/models/config/test_dump_configuration.py
🧠 Learnings (5)
📚 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/api/responses/successful/configuration.pysrc/models/config.py
📚 Learning: 2026-02-25T07:46:33.545Z
Learnt from: asimurka
Repo: lightspeed-core/lightspeed-stack PR: 1211
File: src/models/responses.py:8-16
Timestamp: 2026-02-25T07:46:33.545Z
Learning: In the Python codebase, requests.py should use OpenAIResponseInputTool as Tool while responses.py uses OpenAIResponseTool as Tool. This difference is intentional due to differing schemas for input vs output tools in llama-stack-api. Apply this distinction consistently to other models under src/models (e.g., ensure request-related tools use the InputTool variant and response-related tools use the ResponseTool variant). If adding new tools, choose the corresponding InputTool or Tool class based on whether the tool represents input or output, and document the rationale in code comments.
Applied to files:
src/models/api/responses/successful/configuration.pysrc/models/config.py
📚 Learning: 2026-06-24T13:45:37.249Z
Learnt from: Jdubrick
Repo: lightspeed-core/lightspeed-stack PR: 1971
File: src/utils/markdown_repair.py:31-36
Timestamp: 2026-06-24T13:45:37.249Z
Learning: In the lightspeed-stack repository, docstrings must use the section header name "Parameters:" (not "Args:") for function arguments, even if the project references Google Python docstring conventions. Ensure docstrings follow the project’s established "Parameters:" header format for any documented function parameters.
Applied to files:
src/models/api/responses/successful/configuration.pysrc/models/config.pytests/integration/endpoints/test_config_integration.pytests/unit/models/config/test_observability_configuration.pytests/unit/models/config/test_dump_configuration.py
📚 Learning: 2026-07-06T15:26:18.398Z
Learnt from: Jdubrick
Repo: lightspeed-core/lightspeed-stack PR: 2071
File: src/models/config.py:2416-2422
Timestamp: 2026-07-06T15:26:18.398Z
Learning: In this repo’s Python code under src/**, don’t treat differences in string concatenation style as a style inconsistency when Black has effectively forced (or made clearer) use of explicit `+` string concatenation in multi-line logger/string expressions. If adjacent-literal implicit concatenation is avoided/changed specifically to accommodate Black’s formatting in these call sites, accept the `+` usage and don’t recommend converting it solely for consistency with nearby blocks that use implicit concatenation.
Applied to files:
src/models/api/responses/successful/configuration.pysrc/models/config.py
📚 Learning: 2026-07-17T19:25:05.325Z
Learnt from: Jdubrick
Repo: lightspeed-core/lightspeed-stack PR: 2166
File: src/utils/saved_prompts.py:129-157
Timestamp: 2026-07-17T19:25:05.325Z
Learning: For any endpoint that handles saved prompts and calls `src/utils/saved_prompts.py::create_saved_prompt`, treat the endpoint as the validation boundary. Before calling `create_saved_prompt`, validate the incoming saved-prompt name and content, specifically using `validate_saved_prompt_name` and then persist (store) the normalized value it returns. Do not call `create_saved_prompt` with unvalidated/raw name/content.
Applied to files:
src/models/api/responses/successful/configuration.pysrc/models/config.py
🪛 LanguageTool
docs/LCORE-Strategic-Path-Forward.md
[style] ~4-~4: Some style guides suggest that commas should set off the year in a month-day-year date.
Context: ...Anik Bhattacharjee Date: July 22, 2026 Status: Strategic Proposal - Seek...
(MISSING_COMMA_AFTER_YEAR)
[style] ~166-~166: To elevate your writing, try using more formal phrasing here.
Context: ...e - llama-stack gaps) - "Migration cost keeps growing as we add features" (true - 18 months d...
(CONTINUE_TO_VB)
[style] ~335-~335: Consider using an extreme adjective for ‘small’.
Context: ...irt Sizing) Size Legend: - XS (Extra Small) = Trivial/Exists (0-1 weeks) - S (...
(EXTREME_ADJECTIVES)
[inconsistency] ~550-~550: Did you mean to refer to the current year? July 22, 2026 is not a Tuesday, but a Wednesday.
Context: ...imeline) ### This Week (July 22-26) Tuesday, July 22 (today): - Share document with teams ...
(EN_DATE_WEEKDAY_CURRENTYEAR)
[inconsistency] ~552-~552: Did you mean to refer to the current year? July 26, 2026 is not a Friday, but a Sunday.
Context: ... with teams - Schedule strategy review (Friday, July 26) Friday, July 26: - **Strategy rev...
(EN_DATE_WEEKDAY_CURRENTYEAR)
[inconsistency] ~554-~554: Did you mean to refer to the current year? July 26, 2026 is not a Friday, but a Sunday.
Context: ...le strategy review (Friday, July 26) Friday, July 26: - Strategy review meeting (2 hou...
(EN_DATE_WEEKDAY_CURRENTYEAR)
[inconsistency] ~560-~560: Did you mean to refer to the current year? August 5, 2026 is not a Monday, but a Wednesday.
Context: ...k of August 5: Implementation Starts Monday, August 5: - Fork OLS → lightspeed-core-stack...
(EN_DATE_WEEKDAY_CURRENTYEAR)
[style] ~651-~651: Some style guides suggest that commas should set off the year in a month-day-year date.
Context: ...uperseded Revision Date: July 22, 2026 Next Review: July 26, 2026 (Strat...
(MISSING_COMMA_AFTER_YEAR)
🪛 markdownlint-cli2 (0.23.1)
docs/LCORE-Strategic-Path-Forward.md
[warning] 42-42: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 42-42: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
[warning] 51-51: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 51-51: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
[warning] 98-98: Link fragments should be valid
(MD051, link-fragments)
[warning] 99-99: Link fragments should be valid
(MD051, link-fragments)
[warning] 138-138: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 138-138: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
[warning] 283-283: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
[warning] 314-314: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 314-314: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
[warning] 425-425: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 431-431: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 435-435: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 440-440: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 473-473: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🔇 Additional comments (6)
src/models/config.py (1)
2857-2863: Redact sensitive OTEL header values before serialization.This still copies
OTEL_EXPORTER_OTLP_HEADERSand signal-specific header variables verbatim, despite the comment claiming secrets are excluded. SinceConfigurationis returned by the config response, exporter credentials can be disclosed. This repeats the existing unresolved finding.Source: Coding guidelines
tests/unit/models/config/test_observability_configuration.py (1)
16-17: Document test parameters.These docstrings still omit
Parameters:sections for fixture and parametrized inputs. As per coding guidelines, document function parameters using the repository’sParameters:format.Also applies to: 27-28, 48-51, 100-101
Sources: Coding guidelines, Learnings
docs/devel_doc/openapi.json (1)
6756-6763: LGTM!Also applies to: 12378-12382, 12479-12486, 15131-15146
tests/integration/endpoints/test_config_integration.py (2)
124-167: Exercise the/configendpoint in this test.The test still validates only
ObservabilityConfiguration.from_environment(). Rebuild the configuration after setting the variables, invokeconfig_endpoint_handler, and assert the values through the response so stale endpoint state cannot pass unnoticed. This is the same unresolved issue from the prior review.
3-3: LGTM!Also applies to: 94-123
tests/unit/models/config/test_dump_configuration.py (1)
239-239: LGTM!Also applies to: 467-467, 846-846, 1109-1109, 1405-1405, 1628-1628, 2011-2011, 2240-2240, 2469-2469, 2705-2705
| **Extraction effort**: 8-10 weeks | ||
| **Build-from-scratch effort**: 62 weeks | ||
| **Time advantage**: 6:1 |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Reconcile the build-effort estimates before using the ratio claims.
The document cites 62 weeks for build-from-scratch, then 99 weeks in the comparison table and 120/150-week scenarios. Consequently, both “6:1” and “10x faster” cannot be derived from one consistent baseline. Choose one estimate and recalculate the comparison and decision analysis.
Also applies to: 342-359, 519-523
🤖 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 `@docs/LCORE-Strategic-Path-Forward.md` around lines 35 - 37, Reconcile the
build-from-scratch estimate throughout the document, including the headline,
comparison table, and 120/150-week scenarios, by selecting one consistent
baseline. Recalculate the time-advantage ratio and all “10x faster” or related
comparison claims from that baseline, and update the decision analysis
accordingly.
| ``` | ||
| LCORE (RHEL, Ansible, HCC) OpenShift Lightspeed | ||
| ↓ ↓ | ||
| llama-stack (OGX) LangChain + LlamaIndex | ||
| ↓ ↓ | ||
| Forced rewrite (OGX sunset) Production-ready, 18K LOC | ||
| ``` | ||
|
|
||
| **Tomorrow** (Lightspeed Core Stack 2.0): | ||
| ``` | ||
| Lightspeed Core Stack 2.0 | ||
| (extracted from OLS, 18K LOC) | ||
| ↓ | ||
| ┌───────────┬──────────┬──────────┬──────────┐ | ||
| ↓ ↓ ↓ ↓ ↓ | ||
| OpenShift RHEL LS Ansible LS HCC Agent [Future] | ||
| (plugin) (plugin) (plugin) (plugin) (plugin) | ||
| ``` |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Fix fenced-code formatting before merge.
The markdown linter reports missing language identifiers and blank lines around these fences. Use an appropriate language such as text for diagrams and add the required surrounding blank lines.
Also applies to: 138-146, 283-303, 314-326, 473-477
🧰 Tools
🪛 markdownlint-cli2 (0.23.1)
[warning] 42-42: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 42-42: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
[warning] 51-51: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 51-51: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 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 `@docs/LCORE-Strategic-Path-Forward.md` around lines 42 - 59, Update the fenced
code blocks in docs/LCORE-Strategic-Path-Forward.md, including the diagrams
around the “Lightspeed Core Stack 2.0” section and the other referenced blocks,
by adding an appropriate language identifier such as text and blank lines before
and after each fence. Preserve all diagram content and formatting.
Source: Linters/SAST tools
| ### Timeline | ||
|
|
||
| **Phase 1**: Extract infrastructure (2 weeks) | ||
| **Phase 2**: Abstract authentication (2 weeks) | ||
| **Phase 3**: Product plugin framework (2 weeks) | ||
| **Phase 4**: Migrate all 4 products (4 weeks) | ||
| **Total**: **10 weeks** to unified foundation | ||
|
|
||
| **Meets Gemini deadline** (Oct 16, 2026): ✅ YES (completes mid-September) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Make the schedule dates internally consistent.
A 10-week plan beginning August 5, 2026 ends around October 14, not mid-September. Also, July 22, 2026 was Wednesday, July 26 was Sunday, and August 5 was Wednesday—not Tuesday, Friday, and Monday. Replace the stale “today” reference with an absolute date.
Also applies to: 423-447, 548-564
🤖 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 `@docs/LCORE-Strategic-Path-Forward.md` around lines 61 - 69, Update the
timeline and all referenced date sections to use internally consistent calendar
dates: correct the weekday labels for July 22, July 26, and August 5, replace
any relative “today” wording with an absolute date, and align the 10-week plan
starting August 5, 2026 with its approximately October 14 completion rather than
mid-September. Ensure the Gemini deadline statement and the sections covering
the additional referenced ranges reflect the same schedule.
Source: Linters/SAST tools
| ## Table of Contents | ||
|
|
||
| 1. [The Problem: LCORE's Reinvention Trap](#the-problem-lcores-reinvention-trap) | ||
| 2. [The Opportunity: OLS is Product-Agnostic](#the-opportunity-ols-is-product-agnostic) | ||
| 3. [Extraction Feasibility Analysis](#extraction-feasibility-analysis) | ||
| 4. [Lightspeed Core Stack 2.0 Architecture](#lightspeed-core-stack-20-architecture) | ||
| 5. [Comparison: Extract vs Build-from-Scratch](#comparison-extract-vs-build-from-scratch) | ||
| 6. [Implementation Timeline](#implementation-timeline) | ||
| 7. [Product Migration Strategy](#product-migration-strategy) | ||
| 8. [Cloud Agents: The Differentiation Layer](#cloud-agents-the-differentiation-layer) | ||
| 9. [Risks, Mitigation, and Offramps](#risks-mitigation-and-offramps) | ||
| 10. [Decision Framework and Next Steps](#decision-framework-and-next-steps) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Fix the stale table-of-contents anchors.
The links for “The Problem” and “The Opportunity” point to headings that no longer exist. Update them to match the current heading slugs (#the-problem-lcores-failed-abstraction and the current opportunity heading).
🧰 Tools
🪛 markdownlint-cli2 (0.23.1)
[warning] 98-98: Link fragments should be valid
(MD051, link-fragments)
[warning] 99-99: Link fragments should be valid
(MD051, link-fragments)
🤖 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 `@docs/LCORE-Strategic-Path-Forward.md` around lines 96 - 107, Update the Table
of Contents links for “The Problem” and “The Opportunity” to use the current
heading anchors, replacing the stale problem slug with
`#the-problem-lcores-failed-abstraction` and matching the opportunity link to the
slug generated by the document’s current opportunity heading. Leave the
remaining entries unchanged.
Source: Linters/SAST tools
| ### The Forcing Function: OGX Decommissioning | ||
|
|
||
| **Timeline** (announced July 16, 2026): | ||
| - **OGX 3.5 GA** (Aug 2026) | ||
| - **Praxis 3.6** (Nov 2026) | ||
| - **OGX maintenance mode** (Q1 2027) | ||
| - **OGX decommissioned** (TBD) | ||
|
|
||
| **If we stay on OGX**: 6-9 months of upgrade/migration work, then forced migration to Praxis (another 3-6 months). | ||
|
|
||
| **LCORE's foundation is being sunset.** We must rewrite the LLM integration regardless. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Repository files matching target:"
fd -a 'LCORE-Strategic-Path-Forward\.md$|.*Strategic-Path.*|.*Forward.*' . | sed 's#^\./##'
echo
target="docs/LCORE-Strategic-Path-Forward.md"
if [ -f "$target" ]; then
echo "Target line count:"
wc -l "$target"
echo
echo "Lines 150-220:"
sed -n '150,220p' "$target" | nl -ba -v150
else
echo "Target not found; searching occurrences of OGX decommission/forcing function:"
rg -n "OGX|Decommission|forcing function|Timeline|Praxis 3\.6|maintenance mode" docs . -S || true
fiRepository: lightspeed-core/lightspeed-stack
Length of output: 378
🏁 Script executed:
#!/bin/bash
set -euo pipefail
target="docs/LCORE-Strategic-Path-Forward.md"
echo "Lines 150-220 with line numbers:"
awk 'NR>=150 && NR<=220 {printf "%d %s\n", NR, $0}' "$target"
echo
echo "Occurrences of OGX/decommission/forcing function/context:"
awk '
/OGX|dec[oe]mmissioned|forcing function|Forcing Function|Timeline|Praxis 3\.6|maintenance mode/ {
printf "%d %s\n", NR, $0
}
' "$target"Repository: lightspeed-core/lightspeed-stack
Length of output: 5789
Separate confirmed OGX milestones from the decommissioning assumption.
The Timeline marks OGX decommissioning as “TBD,” but the forcing-function section treats it as established on lines 184 and 192. Reframe those lines to cite only the confirmed July announcement and identify the strategic decision that depends on the unconfirmed decommission date.
🤖 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 `@docs/LCORE-Strategic-Path-Forward.md` around lines 174 - 184, Revise the “The
Forcing Function: OGX Decommissioning” section so it distinguishes confirmed
milestones from the unconfirmed decommissioning date. In the “If we stay on OGX”
and “LCORE’s foundation” statements, cite only the confirmed July announcement
and explicitly frame the strategic decision as contingent on the decommissioning
timeline remaining unconfirmed.
| ### Rediscovering the Original Insight | ||
|
|
||
| **OpenShift Lightspeed is 95% product-agnostic** - just like we thought when we created LCORE. | ||
|
|
||
| **Analysis** (July 2026): | ||
| - **Total codebase**: 18,099 LOC | ||
| - **OpenShift-specific references**: 94 (0.5%) | ||
| - **Product-agnostic code**: ~17,200 LOC (95%) | ||
|
|
||
| ### Where OpenShift Coupling Exists | ||
|
|
||
| Only **3 narrow areas** (1,029 LOC total): | ||
|
|
||
| 1. **K8s Authentication** (359 LOC) - Already has abstraction interface | ||
| 2. **System Prompts** (283 LOC) - Simple templating fixes this | ||
| 3. **Skills Content** (387 LOC) - Product-specific by design |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Define the coupling metric used by the extraction gate.
“94 references,” “95% product-agnostic,” and the “>20% coupling” abort threshold use different implied measurements. Add the reproducible counting method, dependency categories, and denominator so the Week 2 checkpoint can be evaluated objectively.
Also applies to: 398-402, 503-505
🤖 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 `@docs/LCORE-Strategic-Path-Forward.md` around lines 201 - 216, Clarify the
extraction gate’s coupling metric near the “Rediscovering the Original Insight”
analysis and its related checkpoint sections. Define the reproducible counting
method, categorize which dependencies count as OpenShift coupling, state the
denominator used for the percentage, and ensure the “94 references,” “95%
product-agnostic,” and “>20% coupling” threshold use the same metric.
| #### Scenario Payoff Matrix | ||
|
|
||
| | | **Best Case** (P=40%) | **Expected Case** (P=50%) | **Worst Case** (P=10%) | | ||
| |--|---------------|-------------------|----------------| | ||
| | **Extract OLS** | **S** (16 eng-weeks)<br>All features Day 1 | **S** (20 eng-weeks)<br>All features Day 1 | **Abort at Week 2**<br>**XS** (4 eng-weeks sunk)<br>Pivot to Build | | ||
| | **Build from Scratch** | **XXL** (99 eng-weeks)<br>Feature complete | **XXL** (120 eng-weeks)<br>Scope creep | **Architecture failure**<br>**XXL** (150+ eng-weeks sunk)<br>No product, no fallback | | ||
|
|
||
| #### Minimax Calculation | ||
|
|
||
| **Extract OLS**: | ||
| - **Maximum loss** (worst case): **XS** (4 eng-weeks if extraction fails at Week 2 checkpoint) | ||
| - **Recovery path**: Pivot to Build with 2-week delay | ||
| - **Minimax value**: **XS** | ||
|
|
||
| **Build from Scratch**: | ||
| - **Maximum loss** (worst case): **XXL** (150+ eng-weeks if architecture fails after 12+ months) | ||
| - **Recovery path**: None (too late to pivot, product still needed) | ||
| - **Minimax value**: **XXL** | ||
|
|
||
| **Minimax decision**: **Extract OLS** (minimize maximum loss: XS vs XXL) | ||
|
|
||
| **Loss comparison**: Build's worst case is **37.5x larger** than Extract's worst case (4 eng-weeks → 150 eng-weeks). | ||
|
|
||
| #### Why Extraction Satisfies Minimax | ||
|
|
||
| **Extract OLS**: | ||
| 1. **Bounded downside** - Worst case is $16K (early detection at Week 2 gate) | ||
| 2. **Reversible commitment** - Can abort and pivot to Build with minimal sunk cost | ||
| 3. **Early validation** - Week 2 checkpoint tests core assumption (product coupling <20%) | ||
| 4. **Fallback exists** - If extraction fails, Build-from-Scratch is still available | ||
|
|
||
| **Build from Scratch**: | ||
| 1. **Unbounded downside** - Late-stage failures (e.g., "Pydantic AI + MCP doesn't integrate") discovered after months | ||
| 2. **Irreversible commitment** - Too late to pivot after 6-12 months of work | ||
| 3. **Late validation** - Architectural assumptions tested only when system integrates | ||
| 4. **No fallback** - If Build fails, no alternative (OLS extraction opportunity lost) | ||
|
|
||
| #### Minimax Principle Applied | ||
|
|
||
| | Strategy | Worst-Case Loss | Fallback Available? | Decision | | ||
| |----------|----------------|---------------------|----------| | ||
| | **Extract OLS** | $16K | ✅ Yes (pivot to Build) | ✅ **Minimax optimal** | | ||
| | **Build from Scratch** | $600K | ❌ No | ❌ Dominated strategy | | ||
|
|
||
| **Interpretation**: Even if extraction fails completely (10% probability), we lose only $16K before pivoting. But if Build encounters late-stage architectural issues (30% probability based on industry data), we've sunk $600K with no product and no recovery path. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="docs/LCORE-Strategic-Path-Forward.md"
if [ -f "$file" ]; then
echo "== file exists =="
wc -l "$file"
echo "== relevant section =="
sed -n '340,430p' "$file" | nl -ba -v340
else
echo "missing $file"
echo "== candidate files =="
fd -a 'Strategic-Path-Forward.md|LCORE' . | head -50
fiRepository: lightspeed-core/lightspeed-stack
Length of output: 292
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="docs/LCORE-Strategic-Path-Forward.md"
echo "== relevant section with line numbers =="
sed -n '340,430p' "$file" | awk '{printf "%d\t%s\n", NR+339, $0}'
echo
echo "== probability/cost mentions around relevant section =="
sed -n '340,430p' "$file" | grep -nE 'probability|P=|eng-weeks|\\$[0-9]+|loss|minimax|10%|30%|40%|50%|150|16K|600K' || trueRepository: lightspeed-core/lightspeed-stack
Length of output: 7095
Keep the minimax numbers internally consistent.
The scenario row and minimax calculation treat Build’s worst case as the 10% probability, but the interpretation suddenly uses 30% for build failure; it also introduces $16K/$600K dollar values without stating the loaded cost per engineering week. Reconcile the worst-case probability or label it separately, and document the $/week assumption or keep the analysis in eng-weeks.
🤖 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 `@docs/LCORE-Strategic-Path-Forward.md` around lines 373 - 417, The minimax
analysis must use consistent probability and cost units. Update the
“Interpretation” section to retain the documented 10% Build worst-case
probability, or explicitly label 30% as a separate assumption supported
elsewhere; also either state the loaded dollar cost per engineering week before
using $16K/$600K or replace those dollar amounts with the established 4 and 150+
eng-week values. Keep the scenario matrix, minimax calculation, and conclusion
aligned.
| This integration test verifies: | ||
| - OTEL_* environment variables are collected into observability.otel | ||
| - Non-OTEL variables are not included | ||
| - Environment variables set at runtime are reflected in config | ||
|
|
||
| Parameters: | ||
| ---------- | ||
| current_config (AppConfig): Loads root configuration | ||
| test_request (Request): FastAPI request | ||
| test_auth (AuthTuple): noop authentication tuple | ||
| monkeypatch (pytest.MonkeyPatch): Fixture to modify environment variables | ||
| """ | ||
| # pylint: disable=import-outside-toplevel | ||
| from models.config import ObservabilityConfiguration | ||
|
|
||
| # Set OTEL environment variables | ||
| monkeypatch.setenv("OTEL_SDK_DISABLED", "true") | ||
| monkeypatch.setenv("OTEL_SERVICE_NAME", "test-service") | ||
| monkeypatch.setenv("OTEL_EXPORTER_OTLP_ENDPOINT", "http://localhost:4317") | ||
|
|
||
| # Reload configuration to pick up new env vars | ||
| updated_observability = ObservabilityConfiguration.from_environment() | ||
|
|
||
| # Verify OTEL vars are present in the environment | ||
| # (Note: The config is loaded at startup, so these may not be in the response | ||
| # unless we reload the entire config. This test verifies the mechanism works.) | ||
| assert "OTEL_SDK_DISABLED" in os.environ | ||
| assert "OTEL_SERVICE_NAME" in os.environ | ||
| assert "OTEL_EXPORTER_OTLP_ENDPOINT" in os.environ | ||
|
|
||
| # Verify that when we call from_environment(), it picks up the vars | ||
| assert "OTEL_SDK_DISABLED" in updated_observability.otel | ||
| assert updated_observability.otel["OTEL_SDK_DISABLED"] == "true" | ||
| assert "OTEL_SERVICE_NAME" in updated_observability.otel | ||
| assert updated_observability.otel["OTEL_SERVICE_NAME"] == "test-service" |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Assert the complete environment contract.
Add an assertion that OTEL_EXPORTER_OTLP_ENDPOINT appears in updated_observability.otel, and set/assert a non-OTEL variable is excluded. The current test verifies only setup state for those cases.
🤖 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 `@tests/integration/endpoints/test_config_integration.py` around lines 133 -
167, Extend the test around ObservabilityConfiguration.from_environment() to
assert OTEL_EXPORTER_OTLP_ENDPOINT is present in updated_observability.otel with
its configured value, then set a non-OTEL environment variable and assert it is
excluded from updated_observability.otel.
| _DEFAULT_OBSERVABILITY_DUMP: dict[str, dict[str, str]] = { | ||
| "otel": {}, | ||
| } | ||
|
|
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Isolate OTEL environment state in dump tests.
Because Configuration derives observability from OTEL_*, these tests can fail whenever the runner already has an OTEL variable. Clear OTEL_* via a fixture or pass an explicit empty ObservabilityConfiguration before asserting the fixed dump.
🤖 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 `@tests/unit/models/config/test_dump_configuration.py` around lines 47 - 50,
Isolate OTEL environment state in the dump tests by adding a fixture that clears
all OTEL_* variables, or by supplying an explicit empty
ObservabilityConfiguration when constructing Configuration. Ensure assertions
against _DEFAULT_OBSERVABILITY_DUMP remain deterministic regardless of the test
runner environment.
1 participant
Description
Add observability configuration to the v1/config endpoint, exposing all OTEL_* environment variables to provide visibility into the active OpenTelemetry tracing setup.
Type of change
Tools used to create PR
Identify any AI code assistants used in this PR (for transparency and review context)
Related Tickets & Documents
Checklist before requesting a review
Testing
Summary by CodeRabbit
observabilitysection to the configuration response, including anotelblock.OTEL_*environment variables (with sensible defaults when none are set).