LCORE-2311: Added agent streaming query utils#1919
Conversation
|
Warning Review limit reached
More reviews will be available in 54 minutes and 5 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more credits in the billing tab to continue. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. 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 (3)
WalkthroughThis PR introduces a complete agent streaming SSE (Server-Sent Events) module for the ChangesAgent Streaming SSE Implementation and Tests
MCP Tool Result Routing Fix
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 |
| """ | ||
| content = cast(dict[str, Any], part.content) | ||
| if "tools" in content or "error" in content: | ||
| if "tools" in content: |
There was a problem hiding this comment.
Small bug fix in tool processing, error is present in both mcp_list_tools and mcp_call.
![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
🤖 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/utils/agents/streaming.py`:
- Around line 356-371: The base singledispatch function dispatch_stream_event
currently types its first arg as AgentStreamEvent which is too narrow because
agent.run_stream_events() yields AgentRunResultEvent as well; broaden the type
annotation on dispatch_stream_event to accept the full union of possible stream
event types (e.g., include AgentRunResultEvent or a common
supertype/Union[AgentStreamEvent, AgentRunResultEvent]) so the registered
handler for AgentRunResultEvent can be recognized by mypy and the singledispatch
registry; update the function signature and any import/type references
accordingly.
In `@src/utils/agents/tool_processor.py`:
- Around line 482-484: The dispatcher currently chooses
summarize_mcp_list_tools_result vs summarize_mcp_call_result solely by
inspecting content (checking for "tools"), which causes error-only list-tools
payloads to be mis-typed as mcp_call; change the dispatcher to decide by action
context instead of payload shape: add an explicit parameter (e.g., action or
call_action) to the dispatcher function and update its call sites to pass the
original tool action (or, if unavailable, look up the recorded tool-call summary
by tool_call_id) and then call summarize_mcp_list_tools_result(part, tool_round)
when the action is list-tools (even if content is {"error":...}), otherwise call
summarize_mcp_call_result; update function signatures and any callers
accordingly to preserve the mcp_list_tools result.type for failed list-tools
responses.
🪄 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: 8c30708e-17c9-4dbf-bb49-ba2487a2dda3
📒 Files selected for processing (3)
src/utils/agents/streaming.pysrc/utils/agents/tool_processor.pytests/unit/utils/agents/test_streaming.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). (14)
- GitHub Check: Konflux kflux-prd-rh02 / lightspeed-stack-on-pull-request
- GitHub Check: Konflux kflux-prd-rh02 / lightspeed-stack-0-6-on-pull-request
- GitHub Check: spectral
- GitHub Check: build-pr
- GitHub Check: unit_tests (3.12)
- GitHub Check: Pylinter
- GitHub Check: unit_tests (3.13)
- GitHub Check: E2E: server mode / ci / group 2
- GitHub Check: E2E: library 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 3
- GitHub Check: E2E: server mode / ci / group 1
- GitHub Check: E2E Tests for Lightspeed Evaluation job
🧰 Additional context used
📓 Path-based instructions (2)
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/utils/agents/tool_processor.pysrc/utils/agents/streaming.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_streaming.py
🪛 GitHub Actions: Type checks / 0_mypy.txt
src/utils/agents/streaming.py
[error] 293-293: mypy: Argument 1 to "dispatch_stream_event" has incompatible type "PartStartEvent | PartDeltaEvent | PartEndEvent | FinalResultEvent | FunctionToolCallEvent | <6 more items>"; expected "PartStartEvent | PartDeltaEvent | PartEndEvent | FinalResultEvent | FunctionToolCallEvent | FunctionToolResultEvent | OutputToolCallEvent | OutputToolResultEvent | BuiltinToolCallEvent | BuiltinToolResultEvent". [arg-type]
[error] 375-375: mypy: Dispatch type "AgentRunResultEvent[str]" must be subtype of fallback function first argument "PartStartEvent | PartDeltaEvent | PartEndEvent | FinalResultEvent | FunctionToolCallEvent | FunctionToolResultEvent | OutputToolCallEvent | OutputToolResultEvent | BuiltinToolCallEvent | BuiltinToolResultEvent". [misc]
🪛 GitHub Actions: Type checks / mypy
src/utils/agents/streaming.py
[error] 293-293: mypy error: Argument 1 to "dispatch_stream_event" has incompatible type "PartStartEvent | PartDeltaEvent | PartEndEvent | FinalResultEvent | FunctionToolCallEvent | <6 more items>"; expected "PartStartEvent | PartDeltaEvent | PartEndEvent | FinalResultEvent | FunctionToolCallEvent | FunctionToolResultEvent | OutputToolCallEvent | OutputToolResultEvent | BuiltinToolCallEvent | BuiltinToolResultEvent". [arg-type]
[error] 375-375: mypy error: Dispatch type "AgentRunResultEvent[str]" must be subtype of fallback function first argument "PartStartEvent | PartDeltaEvent | PartEndEvent | FinalResultEvent | FunctionToolCallEvent | FunctionToolResultEvent | OutputToolCallEvent | OutputToolResultEvent | BuiltinToolCallEvent | BuiltinToolResultEvent". [misc]
🔇 Additional comments (8)
tests/unit/utils/agents/test_streaming.py (1)
1-991: LGTM!src/utils/agents/streaming.py (7)
1-78: LGTM!
81-132: LGTM!
135-260: LGTM!
263-321: LGTM!
324-353: LGTM!
470-476: ⚡ Quick winInconsistent newline suffix due to operator precedence.
The
oroperator has lower precedence than+, so"\n\n"is only appended whenpart.contentis falsy and the accumulatedtext_partsfallback is used. Ifpart.contenthas a value, no newlines are added. Verify this asymmetry is intentional.# Current behavior: # part.content = "hello" → "hello" (no newlines) # part.content = None → "accumulated text\n\n" (newlines)
506-527: LGTM!
asimurka
force-pushed
the
agent_query_utils
branch
from
eee0e81 to
0990ea5
Compare
|
/retest |
2 participants
Description
Adds agent streaming query core utility functions that shadow original functions.
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
Release Notes
New Features
Bug Fixes
Tests