[LEADS-415] Responses streaming support#255
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (3)
WalkthroughThe streaming parser is refactored from Pydantic-based models to dataclasses, with the monolithic event switch replaced by handler-dispatch tables ( ChangesResponses Endpoint Streaming Support
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 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)
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 |
e8a6dd9 to
5bb24c3
Compare
|
@coderabbitai full review |
✅ Action performedFull review finished. |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/lightspeed_evaluation/core/api/streaming_parser.py (1)
356-358: 💤 Low valueUnreachable code:
argumentscan never beNoneafter line 350.Line 350 uses
or {}as a fallback, soargumentswill always be at least an empty dict. The conditionif arguments is Noneon line 356 will never be true.Consider removing this unreachable block:
Proposed fix
if not tool_name: logger.debug("Tool call missing name/tool_name field") return None - if arguments is None: - logger.debug("Tool call missing args/arguments field for %s", tool_name) - return None - tool_call: dict[str, Any] = {"tool_name": tool_name, "arguments": arguments}🤖 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/lightspeed_evaluation/core/api/streaming_parser.py` around lines 356 - 358, Remove the unreachable `if arguments is None:` condition and its associated debug logging block. Since the arguments variable is assigned with an `or {}` fallback pattern earlier in the code, it will always contain at least an empty dictionary and can never be None, making this entire conditional block dead code that should be deleted.
🤖 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/unit/core/api/test_client_responses.py`:
- Line 1: Remove the module-level pylint disable comment at the top of
test_client_responses.py that suppresses protected-access and duplicate-code
warnings. Instead of disabling these checks globally, identify the specific code
locations that trigger these warnings (likely places where protected members are
accessed with underscore prefixes or where code is duplicated) and either
refactor the code to avoid the issue, or if necessary, apply targeted localized
lint suppressions directly to those specific lines. This ensures code quality
standards are maintained and warnings are addressed rather than hidden.
---
Nitpick comments:
In `@src/lightspeed_evaluation/core/api/streaming_parser.py`:
- Around line 356-358: Remove the unreachable `if arguments is None:` condition
and its associated debug logging block. Since the arguments variable is assigned
with an `or {}` fallback pattern earlier in the code, it will always contain at
least an empty dictionary and can never be None, making this entire conditional
block dead code that should be deleted.
🪄 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: CHILL
Plan: Pro
Run ID: 00d2cc99-273a-4e6c-8365-056cd39844d9
📒 Files selected for processing (19)
config/system.yamlexamples/01_getting_started/basic_setup/README.mdexamples/02_metrics/context_quality/README.mdexamples/02_metrics/conversation_quality/README.mdexamples/02_metrics/keywords_evaluation/README.mdexamples/02_metrics/nlp_metrics/README.mdexamples/02_metrics/response_quality/README.mdexamples/02_metrics/tool_evaluation/README.mdexamples/03_endpoints/responses/README.mdexamples/03_endpoints/responses/eval_data.yamlexamples/03_endpoints/responses/system.yamlsrc/lightspeed_evaluation/core/api/client.pysrc/lightspeed_evaluation/core/api/streaming_parser.pysrc/lightspeed_evaluation/core/constants.pysrc/lightspeed_evaluation/core/metrics/custom/tool_eval.pytests/unit/core/api/conftest.pytests/unit/core/api/test_client_responses.pytests/unit/core/api/test_streaming_parser.pytests/unit/core/metrics/custom/test_tool_eval.py
|
I need to rebase.. |
8c03464 to
54e60be
Compare
|
@coderabbitai full review |
asamal4
left a comment
There was a problem hiding this comment.
Thanks !! Some minor comments - applicable in multiples places
54e60be to
9e9df01
Compare
9e9df01 to
5449b8f
Compare
Description
Followup PR with supporting responses
stream=TrueparameterType 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
/responsesendpoint for real-time delivery[DONE]edge cases)