Skip to content

LCORE-2076: Wired agent skills into request flow#1930

Open
asimurka wants to merge 1 commit into
lightspeed-core:mainfrom
asimurka:wire_agents_skills
Open

LCORE-2076: Wired agent skills into request flow#1930
asimurka wants to merge 1 commit into
lightspeed-core:mainfrom
asimurka:wire_agents_skills

Conversation

@asimurka

@asimurka asimurka commented Jun 15, 2026

Copy link
Copy Markdown
Contributor

Description

Wires skills into query flow by adding it as an agent capability.

Type of change

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

Tools used to create PR

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

  • Assisted-by: Cursor

Related Tickets & Documents

Checklist before requesting a review

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

Testing

  • Please provide detailed steps to perform tests related to this code change.
  • How were the fix/results from this change verified? Please provide relevant screenshots or results.

Summary by CodeRabbit

Release Notes

  • New Features

    • Agent skills are now configurable through application settings, enabling enhanced AI agent capabilities.
  • Tests

    • Extended test coverage for skills configuration and integration functionality.

@coderabbitai

coderabbitai Bot commented Jun 15, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Walkthrough

Adds pydantic-ai-skills>=0.11.0 as a dependency, exposes a new AppConfig.skills property returning Optional[SkillsConfiguration], extends build_agent with private _skills_capability/_agent_capabilities helpers that construct a SkillsCapability from configured paths, and threads configuration.skills into the agent construction call in retrieve_agent_response. Unit tests and shared fixtures are added to cover the new code paths.

Changes

Agent Skills Integration

Layer / File(s) Summary
Dependency and SkillsConfiguration contract
pyproject.toml, src/configuration.py
Adds pydantic-ai-skills>=0.11.0 to project dependencies and introduces AppConfig.skills -> Optional[SkillsConfiguration] property with a LogicError guard when configuration is unloaded.
build_agent skills capability helpers
src/utils/pydantic_ai.py
Imports AgentCapability, SkillsCapability, and SkillsConfiguration; adds _skills_capability and _agent_capabilities private helpers; extends build_agent signature with skills: Optional[SkillsConfiguration]; passes capabilities=_agent_capabilities(skills) to the constructed Agent.
retrieve_agent_response wiring
src/utils/agents/query.py
Passes configuration.skills as the third argument to build_agent in the non-streaming agent construction path.
Unit test fixtures and coverage
tests/unit/conftest.py, tests/unit/utils/agents/test_query.py, tests/unit/utils/test_pydantic_ai.py
Adds mock_client, mock_params, and mock_skills_configuration shared fixtures; adds patch_query_configuration fixture applied to two existing async tests; adds new test classes for _skills_capability, _agent_capabilities, and build_agent capability composition; updates existing build_agent call sites to pass None.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

Suggested reviewers

  • radofuchs
  • tisnik
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'LCORE-2076: Wired agent skills into request flow' clearly summarizes the main change: integrating agent skills into the query request flow. It is concise, specific, and directly related to the primary modifications across all changed files.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
✨ Simplify code
  • Create PR with simplified 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.

❤️ Share

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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 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/configuration.py`:
- Around line 509-513: In the skills method's return type annotation, replace
the Optional type syntax with modern union typing. Change the return type from
Optional[SkillsConfiguration] to SkillsConfiguration | None to align with the
project's required annotation style and coding guidelines for modern type
syntax.

In `@src/utils/pydantic_ai.py`:
- Around line 75-77: Update the type hints in the _skills_capability function
and other affected functions to use modern union syntax instead of Optional.
Replace all instances of Optional[Type] with Type | None throughout the affected
functions. This includes updating the parameter type hints for skills_config and
the return type hint for the _skills_capability function, as well as any other
functions in the same file that use the Optional syntax (as indicated by the
comment applying to multiple locations).
🪄 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

Run ID: a36eb35d-dd2e-450d-a95c-8ba8175fb41a

📥 Commits

Reviewing files that changed from the base of the PR and between 750ff7f and 0468d21.

⛔ Files ignored due to path filters (1)
  • uv.lock is excluded by !**/*.lock
📒 Files selected for processing (7)
  • pyproject.toml
  • src/configuration.py
  • src/utils/agents/query.py
  • src/utils/pydantic_ai.py
  • tests/unit/conftest.py
  • tests/unit/utils/agents/test_query.py
  • tests/unit/utils/test_pydantic_ai.py
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (16)
  • GitHub Check: integration_tests (3.13)
  • GitHub Check: integration_tests (3.12)
  • GitHub Check: mypy
  • GitHub Check: build-pr
  • GitHub Check: unit_tests (3.12)
  • GitHub Check: unit_tests (3.13)
  • GitHub Check: Pylinter
  • GitHub Check: E2E: library mode / ci / group 1
  • GitHub Check: E2E: library mode / ci / group 3
  • GitHub Check: E2E: server mode / ci / group 3
  • GitHub Check: E2E: server mode / ci / group 2
  • GitHub Check: E2E: library mode / ci / group 2
  • GitHub Check: E2E: server mode / ci / group 1
  • GitHub Check: E2E Tests for Lightspeed Evaluation job
  • GitHub Check: Konflux kflux-prd-rh02 / lightspeed-stack-0-6-on-pull-request
  • GitHub Check: Konflux kflux-prd-rh02 / lightspeed-stack-on-pull-request
🧰 Additional context used
📓 Path-based instructions (4)
src/**/*.py

📄 CodeRabbit inference engine (AGENTS.md)

src/**/*.py: Use absolute imports for internal modules: from authentication import get_auth_dependency
Llama Stack imports: Use from llama_stack_client import AsyncLlamaStackClient
Check constants.py for shared constants before defining new ones
All modules must start with descriptive docstrings explaining purpose
Use logger = get_logger(__name__) from log.py for module logging
All functions must have complete type annotations for parameters and return types, use modern syntax (str | int), and include descriptive docstrings
Use snake_case with descriptive, action-oriented names for functions (get_, validate_, check_)
Avoid in-place parameter modification anti-patterns; return new data structures instead of modifying function parameters
Use async def for I/O operations and external API calls
Use standard log levels with clear purposes: debug() for diagnostic info, info() for program execution, warning() for unexpected events, error() for serious problems
All classes must have descriptive docstrings explaining purpose and use PascalCase with standard suffixes: Configuration, Error/Exception, Resolver, Interface
Abstract classes must use ABC with @abstractmethod decorators
Follow Google Python docstring conventions with required sections: Parameters, Returns, Raises, and Attributes for classes

Files:

  • src/utils/agents/query.py
  • src/configuration.py
  • src/utils/pydantic_ai.py
src/**/configuration.py

📄 CodeRabbit inference engine (AGENTS.md)

src/**/configuration.py: All config models must extend ConfigurationBase with extra="forbid" to reject unknown fields
Use @field_validator and @model_validator for custom validation in Pydantic models

Files:

  • src/configuration.py
tests/**/*.py

📄 CodeRabbit inference engine (AGENTS.md)

tests/**/*.py: Use pytest for all unit and integration tests; do not use unittest
Use pytest.mark.asyncio marker for async tests

Files:

  • tests/unit/conftest.py
  • tests/unit/utils/test_pydantic_ai.py
  • tests/unit/utils/agents/test_query.py
tests/**/conftest.py

📄 CodeRabbit inference engine (AGENTS.md)

Use conftest.py for shared pytest fixtures and pytest-mock for AsyncMock objects

Files:

  • tests/unit/conftest.py
🔇 Additional comments (5)
pyproject.toml (1)

83-84: LGTM!

src/utils/agents/query.py (1)

313-313: LGTM!

tests/unit/conftest.py (1)

82-118: LGTM!

tests/unit/utils/agents/test_query.py (1)

119-126: LGTM!

Also applies to: 132-132, 435-436, 462-463

tests/unit/utils/test_pydantic_ai.py (1)

8-20: LGTM!

Also applies to: 205-242, 268-268, 290-290, 313-313, 317-347

Comment thread src/configuration.py
Comment thread src/utils/pydantic_ai.py
@asimurka asimurka requested review from jrobertboos and tisnik June 15, 2026 15:30

@jrobertboos jrobertboos left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants