RHIDP-15891: add /v1/skills endpoint - #2294
Conversation
Signed-off-by: Stephanie <yangcao@redhat.com>
WalkthroughAdds an authenticated ChangesSkills endpoint
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant skills_endpoint_handler
participant Configuration
participant get_skills_metadata
Client->>skills_endpoint_handler: GET /v1/skills
skills_endpoint_handler->>Configuration: validate configuration
skills_endpoint_handler->>get_skills_metadata: load configured skill metadata
get_skills_metadata-->>skills_endpoint_handler: return names and descriptions
skills_endpoint_handler-->>Client: SkillsResponse
Possibly related PRs
Suggested reviewers: 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 |
Signed-off-by: Stephanie <yangcao@redhat.com>
![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/app/endpoints/skills.py`:
- Around line 63-66: Update the skills endpoint handler around
get_skills_metadata so synchronous SkillsCapability discovery and toolset.skills
access never block the FastAPI event loop. Prefer reusing startup-cached
capability data with appropriate configuration invalidation; otherwise offload
get_skills_metadata to a worker thread before constructing SkillsResponse.
In `@src/models/api/responses/successful/catalog.py`:
- Around line 13-15: Replace the untyped skills contract with a shared
SkillMetadata model containing name and description as strings, and use it for
the skills field in src/models/api/responses/successful/catalog.py at lines
13-15. Update the relevant helper return path in
src/utils/pydantic_ai_helpers.py at lines 107-124 to return the same typed
contract, or at minimum list[dict[str, str]], eliminating Any at both sites.
In `@tests/unit/app/endpoints/test_skills.py`:
- Around line 18-43: The existing test_skills_loaded direct-handler coverage
does not validate route registration, serialization, or authorization wiring.
Add a client-level test for the mounted /v1/skills route that verifies an
authorized request succeeds and a denied request is rejected, while retaining
the direct filesystem-based tests for metadata behavior. Ensure the route
remains versioned under /v1 and uses the configured authorization dependency.
🪄 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: 03a85799-5ce2-4f42-bade-0ce9763d1f02
📒 Files selected for processing (9)
src/app/endpoints/skills.pysrc/app/main.pysrc/app/routers.pysrc/models/api/responses/successful/__init__.pysrc/models/api/responses/successful/catalog.pysrc/models/config.pysrc/utils/pydantic_ai_helpers.pytests/unit/app/endpoints/test_skills.pytests/unit/utils/test_pydantic_ai.py
📜 Review details
⏰ Context from checks skipped due to timeout. (14)
- GitHub Check: black
- GitHub Check: bandit
- GitHub Check: build-pr
- GitHub Check: Pylinter
- GitHub Check: integration_tests (3.12)
- GitHub Check: integration_tests (3.13)
- GitHub Check: E2E: server mode / ci / group 1
- GitHub Check: E2E: server mode / ci / group 3
- GitHub Check: E2E: library mode / ci / group 2
- GitHub Check: E2E: library mode / ci / group 1
- GitHub Check: E2E: server mode / ci / group 2
- GitHub Check: E2E: library 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) / 0_spectral.txt: RHIDP-15891: add /v1/skills 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) / spectral: RHIDP-15891: add /v1/skills 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 (6)
**/*
📄 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/app/endpoints/skills.pytests/unit/utils/test_pydantic_ai.pysrc/app/routers.pysrc/app/main.pysrc/utils/pydantic_ai_helpers.pysrc/models/api/responses/successful/catalog.pysrc/models/config.pysrc/models/api/responses/successful/__init__.pytests/unit/app/endpoints/test_skills.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/app/endpoints/skills.pytests/unit/utils/test_pydantic_ai.pysrc/app/routers.pysrc/app/main.pysrc/utils/pydantic_ai_helpers.pysrc/models/api/responses/successful/catalog.pysrc/models/config.pysrc/models/api/responses/successful/__init__.pytests/unit/app/endpoints/test_skills.py
src/app/endpoints/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Use FastAPI
HTTPExceptionwith appropriate status codes for API endpoints.
Files:
src/app/endpoints/skills.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/utils/test_pydantic_ai.pytests/unit/app/endpoints/test_skills.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/catalog.pysrc/models/config.pysrc/models/api/responses/successful/__init__.py
**/__init__.py
📄 CodeRabbit inference engine (AGENTS.md)
Package
__init__.pyfiles must contain brief package descriptions.
Files:
src/models/api/responses/successful/__init__.py
🧠 Learnings (7)
📚 Learning: 2026-04-06T20:18:07.852Z
Learnt from: major
Repo: lightspeed-core/lightspeed-stack PR: 1463
File: src/app/endpoints/rlsapi_v1.py:266-271
Timestamp: 2026-04-06T20:18:07.852Z
Learning: In the lightspeed-stack codebase, within `src/app/endpoints/` inference/MCP endpoints, treat `tools: Optional[list[Any]]` in MCP tool definitions as an intentional, consistent typing pattern (used across `query`, `responses`, `streaming_query`, `rlsapi_v1`). Do not raise or suggest this as a typing issue during code review; changing it in isolation could break endpoint typing consistency across the codebase.
Applied to files:
src/app/endpoints/skills.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/app/endpoints/skills.pytests/unit/utils/test_pydantic_ai.pysrc/app/routers.pysrc/app/main.pysrc/utils/pydantic_ai_helpers.pysrc/models/api/responses/successful/catalog.pysrc/models/config.pysrc/models/api/responses/successful/__init__.pytests/unit/app/endpoints/test_skills.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/app/endpoints/skills.pysrc/app/routers.pysrc/app/main.pysrc/utils/pydantic_ai_helpers.pysrc/models/api/responses/successful/catalog.pysrc/models/config.pysrc/models/api/responses/successful/__init__.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/app/endpoints/skills.pysrc/app/routers.pysrc/app/main.pysrc/utils/pydantic_ai_helpers.pysrc/models/api/responses/successful/catalog.pysrc/models/config.pysrc/models/api/responses/successful/__init__.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/api/responses/successful/catalog.pysrc/models/config.pysrc/models/api/responses/successful/__init__.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/catalog.pysrc/models/config.pysrc/models/api/responses/successful/__init__.py
📚 Learning: 2026-07-21T11:10:05.060Z
Learnt from: are-ces
Repo: lightspeed-core/lightspeed-stack PR: 2162
File: src/a2a_client/__init__.py:3-9
Timestamp: 2026-07-21T11:10:05.060Z
Learning: In this repository, it is acceptable for Python package `__init__.py` files to contain functional code (not only docstrings/metadata) and to perform package-level re-exports. Do not flag `__init__.py` solely for containing imports or other logic used to re-export symbols; this is allowed when it’s implemented via imports and `__all__` (or otherwise clearly intended to define the package’s public API).
Applied to files:
src/models/api/responses/successful/__init__.py
🔇 Additional comments (5)
src/models/api/responses/successful/__init__.py (1)
10-10: LGTM!Also applies to: 104-104
src/models/config.py (1)
1276-1276: LGTM!src/app/routers.py (1)
30-30: LGTM!Also applies to: 59-59
src/app/main.py (1)
66-66: LGTM!tests/unit/utils/test_pydantic_ai.py (1)
18-18: LGTM!Also applies to: 187-205
| @pytest.mark.asyncio | ||
| async def test_skills_loaded( | ||
| mocker: MockerFixture, | ||
| tmp_path: Path, | ||
| ) -> None: | ||
| """Test that loaded skills are returned with name and description.""" | ||
| mock_authorization_resolvers(mocker) | ||
|
|
||
| skills_root = tmp_path / "skills" | ||
| for name, desc in [ | ||
| ("code-review", "Review code for quality and security"), | ||
| ("openshift-troubleshooting", "Troubleshoot OpenShift cluster issues"), | ||
| ]: | ||
| skill_dir = skills_root / name | ||
| skill_dir.mkdir(parents=True) | ||
| (skill_dir / "SKILL.md").write_text( | ||
| f"---\nname: {name}\ndescription: {desc}\n---\n\nInstructions.\n", | ||
| encoding="utf-8", | ||
| ) | ||
|
|
||
| skills_config = SkillsConfiguration(paths=[skills_root]) | ||
| mock_config = mocker.patch("app.endpoints.skills.configuration") | ||
| mock_config.configuration.skills = skills_config | ||
|
|
||
| request = Request(scope={"type": "http"}) | ||
| response = await skills_endpoint_handler(auth=MOCK_AUTH, request=request) |
There was a problem hiding this comment.
🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick win
Add route-level authorization coverage.
These tests call the handler directly with a hand-built request and auth tuple, so they do not verify /v1/skills registration, FastAPI serialization, or dependency wiring. Add at least one client-level test covering the mounted route and denied access; keep the direct filesystem tests for metadata cases.
Based on the PR objective, this endpoint must be both versioned and authorized.
🤖 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/app/endpoints/test_skills.py` around lines 18 - 43, The existing
test_skills_loaded direct-handler coverage does not validate route registration,
serialization, or authorization wiring. Add a client-level test for the mounted
/v1/skills route that verifies an authorized request succeeds and a denied
request is rejected, while retaining the direct filesystem-based tests for
metadata behavior. Ensure the route remains versioned under /v1 and uses the
configured authorization dependency.
Signed-off-by: Stephanie <yangcao@redhat.com>
fdb284a to
450a1d3
Compare
Signed-off-by: Stephanie <yangcao@redhat.com>
asimurka
left a comment
asimurka
left a comment
There was a problem hiding this comment.
Skills catalog is already available under GET /tools endpoint. You can filter them on provider_id == "agent-skills".
|
@yangcao77 please look at our contributing guide https://github.com/lightspeed-core/lightspeed-stack/blob/main/CONTRIBUTING.md . It basically says, that features as this one needs to be a bit coordinated. And all such changes should include documentation (user+devel one) and ideally also integration and end to end tests. We are forced to require all these things on order to make sure the LCore will be in a good shape, prepared to be released, and also able to pass all security checks we are asked to pass. @sbunciak FYI |
|
@asimurka thanks for taking a look.
|
Signed-off-by: Stephanie <yangcao@redhat.com>
|
Thanks for the pointer, @tisnik I've added: Docs: |
…to list-skills-endpoint
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 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/devel_doc/ARCHITECTURE.md`:
- Around line 506-508: Update the “List Skills” endpoint documentation to use
the versioned path GET /v1/skills instead of GET /skills, while preserving the
existing description of its behavior.
In `@docs/user_doc/skills_guide.md`:
- Around line 211-219: Update the “Inspecting Loaded Skills via REST API” curl
example to reflect the handler’s authentication requirements by showing the
necessary credentials and permissions, or explicitly state that authentication
must be disabled for the unauthenticated request to work.
In `@README.md`:
- Line 1273: Add a blank line between the **Response Body:** paragraph and the
following ```json fenced block in the README.md section.
- Around line 1268-1270: Update the README curl example to include the required
authorization header, and document that requests without valid authentication
may return 401 or 403 when authentication is enabled.
In `@src/models/api/responses/successful/catalog.py`:
- Around line 11-12: Update the SkillsResponse class docstring to include an
Attributes section documenting the skills field, following the project's
existing docstring conventions for public response models.
In `@tests/integration/endpoints/test_skills_integration.py`:
- Around line 23-30: Complete the _write_skill docstring by adding Parameters
and Returns sections that document skills_root, name, and description, and state
that the helper returns None. Keep the existing behavior and summary unchanged.
🪄 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: 52979f47-1a4d-4db0-9293-55d50b480683
📒 Files selected for processing (16)
README.mddocs/devel_doc/ARCHITECTURE.mddocs/devel_doc/openapi.jsondocs/models/responses_succ.mddocs/user_doc/skills_guide.mdsrc/app/endpoints/skills.pysrc/models/api/responses/successful/catalog.pysrc/models/common/__init__.pysrc/models/common/skills.pysrc/utils/models_dumper.pysrc/utils/pydantic_ai_helpers.pytests/e2e/features/skills.featuretests/integration/endpoints/test_skills_integration.pytests/unit/app/endpoints/test_skills.pytests/unit/app/test_routers.pytests/unit/utils/test_pydantic_ai.py
📜 Review details
⏰ Context from checks skipped due to timeout. (8)
- GitHub Check: E2E Tests for Lightspeed Evaluation job
- GitHub Check: E2E: library mode / ci / group 2
- GitHub Check: E2E: server mode / ci / group 1
- GitHub Check: E2E: server mode / ci / group 3
- GitHub Check: E2E: server mode / ci / group 2
- GitHub Check: E2E: library mode / ci / group 3
- GitHub Check: E2E: library mode / ci / group 1
- GitHub Check: Konflux kflux-prd-rh02 / lightspeed-stack-0-7-on-pull-request
🧰 Additional context used
📓 Path-based instructions (8)
**/*
📄 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:
docs/devel_doc/ARCHITECTURE.mdsrc/utils/models_dumper.pysrc/models/common/skills.pytests/e2e/features/skills.featuredocs/models/responses_succ.mdtests/integration/endpoints/test_skills_integration.pytests/unit/app/test_routers.pydocs/user_doc/skills_guide.mdREADME.mdsrc/models/common/__init__.pydocs/devel_doc/openapi.jsonsrc/utils/pydantic_ai_helpers.pysrc/app/endpoints/skills.pytests/unit/utils/test_pydantic_ai.pytests/unit/app/endpoints/test_skills.pysrc/models/api/responses/successful/catalog.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/utils/models_dumper.pysrc/models/common/skills.pytests/integration/endpoints/test_skills_integration.pytests/unit/app/test_routers.pysrc/models/common/__init__.pysrc/utils/pydantic_ai_helpers.pysrc/app/endpoints/skills.pytests/unit/utils/test_pydantic_ai.pytests/unit/app/endpoints/test_skills.pysrc/models/api/responses/successful/catalog.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/common/skills.pysrc/models/common/__init__.pysrc/models/api/responses/successful/catalog.py
tests/e2e/features/**/*.feature
📄 CodeRabbit inference engine (AGENTS.md)
End-to-end scenarios must use behave’s Gherkin feature-file format.
Files:
tests/e2e/features/skills.feature
tests/integration/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Use pytest for all integration tests; do not use unittest.
Files:
tests/integration/endpoints/test_skills_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/app/test_routers.pytests/unit/utils/test_pydantic_ai.pytests/unit/app/endpoints/test_skills.py
**/__init__.py
📄 CodeRabbit inference engine (AGENTS.md)
Package
__init__.pyfiles must contain brief package descriptions.
Files:
src/models/common/__init__.py
src/app/endpoints/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Use FastAPI
HTTPExceptionwith appropriate status codes for API endpoints.
Files:
src/app/endpoints/skills.py
🧠 Learnings (7)
📚 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/utils/models_dumper.pysrc/models/common/skills.pytests/integration/endpoints/test_skills_integration.pytests/unit/app/test_routers.pysrc/models/common/__init__.pysrc/utils/pydantic_ai_helpers.pysrc/app/endpoints/skills.pytests/unit/utils/test_pydantic_ai.pytests/unit/app/endpoints/test_skills.pysrc/models/api/responses/successful/catalog.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/utils/models_dumper.pysrc/models/common/skills.pysrc/models/common/__init__.pysrc/utils/pydantic_ai_helpers.pysrc/app/endpoints/skills.pysrc/models/api/responses/successful/catalog.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/utils/models_dumper.pysrc/models/common/skills.pysrc/models/common/__init__.pysrc/utils/pydantic_ai_helpers.pysrc/app/endpoints/skills.pysrc/models/api/responses/successful/catalog.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/common/skills.pysrc/models/common/__init__.pysrc/models/api/responses/successful/catalog.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/common/skills.pysrc/models/common/__init__.pysrc/models/api/responses/successful/catalog.py
📚 Learning: 2026-07-21T11:10:05.060Z
Learnt from: are-ces
Repo: lightspeed-core/lightspeed-stack PR: 2162
File: src/a2a_client/__init__.py:3-9
Timestamp: 2026-07-21T11:10:05.060Z
Learning: In this repository, it is acceptable for Python package `__init__.py` files to contain functional code (not only docstrings/metadata) and to perform package-level re-exports. Do not flag `__init__.py` solely for containing imports or other logic used to re-export symbols; this is allowed when it’s implemented via imports and `__all__` (or otherwise clearly intended to define the package’s public API).
Applied to files:
src/models/common/__init__.py
📚 Learning: 2026-04-06T20:18:07.852Z
Learnt from: major
Repo: lightspeed-core/lightspeed-stack PR: 1463
File: src/app/endpoints/rlsapi_v1.py:266-271
Timestamp: 2026-04-06T20:18:07.852Z
Learning: In the lightspeed-stack codebase, within `src/app/endpoints/` inference/MCP endpoints, treat `tools: Optional[list[Any]]` in MCP tool definitions as an intentional, consistent typing pattern (used across `query`, `responses`, `streaming_query`, `rlsapi_v1`). Do not raise or suggest this as a typing issue during code review; changing it in isolation could break endpoint typing consistency across the codebase.
Applied to files:
src/app/endpoints/skills.py
🪛 markdownlint-cli2 (0.23.1)
README.md
[warning] 1273-1273: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
🔇 Additional comments (14)
src/models/common/skills.py (1)
1-17: LGTM!src/models/common/__init__.py (1)
21-21: LGTM!Also applies to: 52-52
src/models/api/responses/successful/catalog.py (1)
8-8: LGTM!Also applies to: 14-35
src/utils/models_dumper.py (1)
76-76: LGTM!Also applies to: 120-120
docs/user_doc/skills_guide.md (1)
19-19: LGTM!docs/models/responses_succ.md (1)
2137-2152: LGTM!Also applies to: 2173-2183
docs/devel_doc/openapi.json (1)
1674-1823: LGTM!Also applies to: 11799-11799, 20949-20969, 20987-21018, 22599-22602
tests/e2e/features/skills.feature (1)
183-234: Add denied-access coverage.The scenarios assert only successful responses. Add unauthenticated and unauthorized requests that assert the documented
401and403responses.src/utils/pydantic_ai_helpers.py (1)
15-15: LGTM!Also applies to: 108-125
src/app/endpoints/skills.py (1)
6-6: LGTM!Also applies to: 39-69
tests/unit/app/endpoints/test_skills.py (1)
47-51: LGTM!Also applies to: 97-119
tests/unit/utils/test_pydantic_ai.py (1)
187-203: LGTM!tests/integration/endpoints/test_skills_integration.py (1)
33-87: LGTM!tests/unit/app/test_routers.py (1)
33-33: LGTM!Also applies to: 126-134, 169-177
Signed-off-by: Stephanie <yangcao@redhat.com>
Signed-off-by: Stephanie <yangcao@redhat.com>
There was a problem hiding this comment.
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)
3050-3055: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winDocument
Configuration.observability.The new public field is not described in
Configuration's class docstring. Add anAttributes:entry forobservabilityso the configuration model documentation remains complete.As per coding guidelines, class docstrings must document applicable
Attributessections.🤖 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 `@src/models/config.py` around lines 3050 - 3055, Add an Attributes entry for the public Configuration.observability field in the Configuration class docstring, describing its OpenTelemetry and observability settings sourced from OTEL_* environment variables. Preserve the existing docstring structure and document only this newly added field.Source: Coding guidelines
♻️ Duplicate comments (1)
README.md (1)
1268-1274: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winShow credentials in the authenticated
curlexample.The section documents authentication, but the copy-paste command sends no
Authorizationheader. It fails with401or403when authentication is enabled. Add a placeholder bearer header, or label the command as valid only when authentication is disabled.Proposed fix
-curl http://localhost:8080/v1/skills +curl -H "Authorization: Bearer <token>" \ + http://localhost:8080/v1/skills🤖 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 `@README.md` around lines 1268 - 1274, Update the authenticated curl example in the authentication documentation to include a placeholder Authorization bearer header, ensuring the copy-paste command works when authentication is enabled.
🤖 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.
Outside diff comments:
In `@src/models/config.py`:
- Around line 3050-3055: Add an Attributes entry for the public
Configuration.observability field in the Configuration class docstring,
describing its OpenTelemetry and observability settings sourced from OTEL_*
environment variables. Preserve the existing docstring structure and document
only this newly added field.
---
Duplicate comments:
In `@README.md`:
- Around line 1268-1274: Update the authenticated curl example in the
authentication documentation to include a placeholder Authorization bearer
header, ensuring the copy-paste command works when authentication is enabled.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 5ba6b65c-74aa-4f8e-94ee-862370e32f86
📒 Files selected for processing (8)
README.mddocs/devel_doc/openapi.jsondocs/models/successful_responses.mddocs/user_doc/skills_guide.mdsrc/models/api/responses/successful/catalog.pysrc/models/config.pysrc/utils/models_dumper.pytests/integration/endpoints/test_skills_integration.py
📜 Review details
⏰ Context from checks skipped due to timeout. (13)
- GitHub Check: integration_tests (3.12)
- GitHub Check: spectral
- GitHub Check: integration_tests (3.13)
- GitHub Check: pydocstyle
- GitHub Check: bandit
- GitHub Check: E2E Tests for Lightspeed Evaluation job
- GitHub Check: E2E: library mode / ci / group 3
- GitHub Check: E2E: server mode / ci / group 3
- GitHub Check: E2E: library mode / ci / group 2
- 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: Konflux kflux-prd-rh02 / lightspeed-stack-0-7-on-pull-request
🧰 Additional context used
📓 Path-based instructions (4)
**/*
📄 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:
docs/models/successful_responses.mdREADME.mddocs/devel_doc/openapi.jsontests/integration/endpoints/test_skills_integration.pydocs/user_doc/skills_guide.mdsrc/models/api/responses/successful/catalog.pysrc/models/config.pysrc/utils/models_dumper.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:
tests/integration/endpoints/test_skills_integration.pysrc/models/api/responses/successful/catalog.pysrc/models/config.pysrc/utils/models_dumper.py
tests/integration/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Use pytest for all integration tests; do not use unittest.
Files:
tests/integration/endpoints/test_skills_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/catalog.pysrc/models/config.py
🧠 Learnings (5)
📚 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:
tests/integration/endpoints/test_skills_integration.pysrc/models/api/responses/successful/catalog.pysrc/models/config.pysrc/utils/models_dumper.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/api/responses/successful/catalog.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/catalog.pysrc/models/config.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/catalog.pysrc/models/config.pysrc/utils/models_dumper.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/catalog.pysrc/models/config.pysrc/utils/models_dumper.py
🔇 Additional comments (11)
src/models/api/responses/successful/catalog.py (1)
12-20: LGTM!Also applies to: 22-39
src/utils/models_dumper.py (1)
187-187: LGTM!Also applies to: 207-224
src/models/config.py (2)
5-5: LGTM!Also applies to: 2777-2785, 2842-2886
2787-2840: 🩺 Stability & AvailabilityNo change needed.
Configuration.observability.otelis only populated for the config response, documented as redacted, and there is no runtime OpenTelemetry initialization path reading it.tests/integration/endpoints/test_skills_integration.py (1)
23-39: LGTM!README.md (1)
1251-1266: LGTM!Also applies to: 1276-1301
docs/devel_doc/openapi.json (2)
6938-6946: LGTM!Also applies to: 13179-13183, 13280-13288, 15947-15962, 21042-21042
1674-1823: 🗄️ Data Integrity & IntegrationNo change needed.
The
/v1/skillsOpenAPI operation uses theget_skillspermission, referencesSkillsResponse, and returnsSkillMetadataitems under theskillstag.docs/user_doc/skills_guide.md (2)
219-221: Make the curl example valid for the authenticated endpoint.The command sends no credentials. Add an authorization header with a token that has the required permission, or state that the command only works when authentication is disabled. The current prose does not make the copy-paste example executable in the documented authenticated setup.
19-19: LGTM!docs/models/successful_responses.md (1)
2308-2323: LGTM!Also applies to: 2344-2354
3 participants
Description
This PR creates a new endpoint
/v1/skillsto exposes the metadata (name, description) of all loaded skills as a JSON response.so consumers like RHDH Intelligent assistant is able to call the REST API to show available skills for user's intention to call.
unit tests also included in this PR
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
New Features
GET /v1/skillsendpoint to list configured skills and their names and descriptions.Documentation
Tests