LCORE-2310: Query agent run#1865
Conversation
|
Warning Review limit reached
More reviews will be available in 41 minutes and 49 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, the refill rate gradually slows as usage increases. The highest same-day bursts are limited more strictly. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (7)
WalkthroughThe ChangesQuery endpoint agent-based retrieval, topic summary delegation, MCP dispatch fix, and test migration
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
asimurka
force-pushed
the
query_agent_run
branch
2 times, most recently
from
7e4d7ef to
ff3dc97
Compare
|
Will be rebased on #1880 |
asimurka
force-pushed
the
query_agent_run
branch
2 times, most recently
from
0831831 to
5efdc0e
Compare
| responses_params: ResponsesApiParams, | ||
| moderation_result: ShieldModerationResult, | ||
| endpoint_path: str, | ||
| _original_input: Optional[ResponseInput] = None, |
There was a problem hiding this comment.
Compacted turn persistence is disconnected and will be part of a separate PR as it will require additional logic.
asimurka
force-pushed
the
query_agent_run
branch
from
5efdc0e to
f2be235
Compare
asimurka
force-pushed
the
query_agent_run
branch
from
f2be235 to
be9f7dd
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/utils/agents/query.py (1)
304-310:⚠️ Potential issue | 🟠 Major | ⚡ Quick winUse the original pre-rewrite input when persisting blocked turns.
Line 304 handles moderation-blocked persistence, but it always writes
responses_params.input. When_original_inputis provided (Line 285), this can persist rewritten synthetic input instead of the user’s actual query.Proposed fix
async def retrieve_agent_response( @@ ) -> TurnSummary: @@ - if moderation_result.decision == "blocked": + turn_input = ( + _original_input if _original_input is not None else responses_params.input + ) + if moderation_result.decision == "blocked": await append_turn_items_to_conversation( client, responses_params.conversation, - responses_params.input, + turn_input, [moderation_result.refusal_response], )🤖 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/utils/agents/query.py` around lines 304 - 310, When persisting a moderation-blocked turn in the condition checking if moderation_result.decision == "blocked", the append_turn_items_to_conversation call currently passes responses_params.input which may be a rewritten or synthetic version. Instead, use the original pre-rewrite input when available. Check if _original_input is provided and pass that to append_turn_items_to_conversation instead of responses_params.input to ensure the user's actual query is persisted rather than any rewritten version.
🤖 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 `@tests/integration/conftest.py`:
- Around line 271-275: The NativeToolReturnPart instantiation in the
create_mcp_list_tools_agent_run_result function is missing an explicit error
field. Since MCP tool-result handling uses error is None to classify success or
failure, add error=None as an explicit parameter to the NativeToolReturnPart
constructor to match the runtime contract and ensure the integration fixture
accurately represents the expected payload structure.
In `@tests/unit/utils/agents/test_tool_processor.py`:
- Around line 499-510: The test
test_mcp_call_with_error_field_not_routed_to_list_tools currently uses a payload
with both output and error fields, but the test name and intent suggest it
should specifically test the error-only edge case. In the content dictionary
passed to NativeToolReturnPart, remove the output field and instead set error to
an actual error message or exception object (rather than None) to create a truly
error-only payload. This will properly exercise the misrouting edge case the
test is meant to guard against, rather than duplicating coverage of mixed
output/error scenarios already tested elsewhere.
---
Outside diff comments:
In `@src/utils/agents/query.py`:
- Around line 304-310: When persisting a moderation-blocked turn in the
condition checking if moderation_result.decision == "blocked", the
append_turn_items_to_conversation call currently passes responses_params.input
which may be a rewritten or synthetic version. Instead, use the original
pre-rewrite input when available. Check if _original_input is provided and pass
that to append_turn_items_to_conversation instead of responses_params.input to
ensure the user's actual query is persisted rather than any rewritten version.
🪄 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: a69df830-70ba-4415-b592-cb7459c4ef5e
📒 Files selected for processing (7)
src/app/endpoints/query.pysrc/utils/agents/query.pytests/integration/conftest.pytests/integration/endpoints/test_query_byok_integration.pytests/integration/endpoints/test_query_integration.pytests/unit/app/endpoints/test_query.pytests/unit/utils/agents/test_tool_processor.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). (13)
- GitHub Check: E2E: server mode / ci / group 1
- GitHub Check: E2E: library mode / ci / group 1
- 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: server mode / ci / group 2
- GitHub Check: integration_tests (3.13)
- GitHub Check: integration_tests (3.12)
- GitHub Check: unit_tests (3.12)
- GitHub Check: E2E Tests for Lightspeed Evaluation job
- GitHub Check: build-pr
- 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: Usefrom llama_stack_client import AsyncLlamaStackClient
Checkconstants.pyfor shared constants before defining new ones
All modules must start with descriptive docstrings explaining purpose
Uselogger = get_logger(__name__)fromlog.pyfor 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
Useasync deffor 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@abstractmethoddecorators
Follow Google Python docstring conventions with required sections: Parameters, Returns, Raises, and Attributes for classes
Files:
src/app/endpoints/query.pysrc/utils/agents/query.py
src/app/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
src/app/**/*.py: FastAPI dependencies: Import fromfastapimodule forAPIRouter,HTTPException,Request,status,Depends
Use FastAPIHTTPExceptionwith appropriate status codes for API endpoints and handleAPIConnectionErrorfrom Llama Stack
Files:
src/app/endpoints/query.py
tests/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
tests/**/*.py: Use pytest for all unit and integration tests; do not use unittest
Usepytest.mark.asynciomarker for async tests
Files:
tests/unit/utils/agents/test_tool_processor.pytests/unit/app/endpoints/test_query.pytests/integration/conftest.pytests/integration/endpoints/test_query_byok_integration.pytests/integration/endpoints/test_query_integration.py
tests/**/conftest.py
📄 CodeRabbit inference engine (AGENTS.md)
Use
conftest.pyfor shared pytest fixtures andpytest-mockfor AsyncMock objects
Files:
tests/integration/conftest.py
🧠 Learnings (2)
📚 Learning: 2026-01-14T09:37:51.612Z
Learnt from: asimurka
Repo: lightspeed-core/lightspeed-stack PR: 988
File: src/app/endpoints/query.py:319-339
Timestamp: 2026-01-14T09:37:51.612Z
Learning: In the lightspeed-stack repository, when provider_id == "azure", the Azure provider with provider_type "remote::azure" is guaranteed to be present in the providers list. Therefore, avoid defensive StopIteration handling for next() when locating the Azure provider in providers within src/app/endpoints/query.py. This change applies specifically to this file (or nearby provider lookup code) and relies on the invariant that the Azure provider exists; if the invariant could be violated, keep the existing StopIteration handling.
Applied to files:
src/app/endpoints/query.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/query.py
🔇 Additional comments (7)
src/utils/agents/query.py (1)
6-7: LGTM!Also applies to: 36-37, 285-297
src/app/endpoints/query.py (1)
18-19: LGTM!Also applies to: 46-47, 73-75, 231-262, 308-311
tests/unit/app/endpoints/test_query.py (1)
130-132: LGTM!Also applies to: 156-164, 251-253, 323-325, 379-381, 400-408, 468-474, 489-489, 520-522, 554-562
tests/unit/utils/agents/test_tool_processor.py (1)
433-491: LGTM!Also applies to: 588-589, 608-609
tests/integration/conftest.py (1)
11-26: LGTM!Also applies to: 169-270, 276-348, 708-717
tests/integration/endpoints/test_query_byok_integration.py (1)
19-23: LGTM!Also applies to: 94-97, 120-125, 138-152, 173-179, 196-216, 288-336, 404-419, 475-491, 748-804, 846-864, 933-950, 1038-1054, 1130-1146, 1240-1255
tests/integration/endpoints/test_query_integration.py (1)
27-31: LGTM!Also applies to: 47-68, 94-122, 165-191, 234-258, 365-390, 439-480, 499-539, 558-582, 604-657, 663-719, 730-752, 782-831, 855-877, 909-940, 977-1000, 1030-1068, 1091-1134, 1140-1172, 1195-1230, 1262-1291, 1329-1353, 1421-1468
asimurka
force-pushed
the
query_agent_run
branch
from
be9f7dd to
4382014
Compare
asimurka
force-pushed
the
query_agent_run
branch
from
4382014 to
70949de
Compare
1 participant
Description
Wired pydantic agents into non-streaming query flow.
Conversation compaction is temporarily disconnected and will be reconnected in a follow-up 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