Refactoring: table based branching #472
Conversation
|
Warning Rate limit exceeded@tisnik has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 6 minutes and 19 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, 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 have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
WalkthroughRefactors stream_build_event in Changes
Sequence Diagram(s)sequenceDiagram
participant C as Caller
participant S as stream_build_event
participant H1 as _handle_turn_start_event
participant H2 as _handle_turn_complete_event
participant H3 as _handle_shield_event
participant H4 as _handle_inference_event
participant H5 as _handle_tool_execution_event
participant H6 as _handle_heartbeat_event
participant L as logger
C->>S: stream_build_event(chunk, event_type, step_type, ...)
Note over S: match (event_type, step_type)
alt (turn_start | turn_awaiting_input), _
S->>H1: delegate to turn-start handler
else turn_complete, _
S->>H2: delegate to turn-complete handler
else _, "shield_call"
S->>H3: delegate to shield handler
else _, "inference"
S->>H4: delegate to inference handler
else _, "tool_execution"
S->>H5: delegate to tool-execution handler
else
S->>L: logger.debug("Unhandled event combo: event_type=%s, step_type=%s")
S->>H6: delegate to heartbeat handler
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
src/app/endpoints/streaming_query.py (3)
157-171: Consolidate match-cases, prefer tuple pattern, and log unexpected combinationsNice readability win. Minor cleanups:
- Use a tuple pattern instead of constructing a list.
- Merge the two identical turn-start cases with an OR-pattern.
- Log unhandled (event_type, step_type) combos before falling back to heartbeat.
- match [event_type, step_type]: - case ["turn_start", _]: + match (event_type, step_type): + case (("turn_start" | "turn_awaiting_input"), _): yield from _handle_turn_start_event(chunk_id) - case ["turn_awaiting_input", _]: - yield from _handle_turn_start_event(chunk_id) - case ["turn_complete", _]: + case ("turn_complete", _): yield from _handle_turn_complete_event(chunk, chunk_id) - case [_, "shield_call"]: + case (_, "shield_call"): yield from _handle_shield_event(chunk, chunk_id) - case [_, "inference"]: + case (_, "inference"): yield from _handle_inference_event(chunk, chunk_id) - case [_, "tool_execution"]: + case (_, "tool_execution"): yield from _handle_tool_execution_event(chunk, chunk_id, metadata_map) case _: - yield from _handle_heartbeat_event(chunk_id) + logger.debug( + "Unhandled event combo: event_type=%s, step_type=%s", + event_type, step_type, + ) + yield from _handle_heartbeat_event(chunk_id)
137-150: Docstring is stale vs. current dispatchThe docstring says it handles only step_progress/step_complete, but the dispatcher now routes turn_start/turn_awaiting_input/turn_complete and step_* by step_type.
- This function processes chunks from the Llama Stack streaming response and formats - them into Server-Sent Events (SSE) format for the client. It handles two main - event types: - - 1. step_progress: Contains text deltas from the model inference process - 2. step_complete: Contains information about completed tool execution steps + This function processes chunks from the Llama Stack streaming response and formats + them into Server-Sent Events (SSE). It dispatches on (event_type, step_type): + - turn_start, turn_awaiting_input -> start token + - turn_complete -> final output message + - step_* with step_type in {"shield_call", "inference", "tool_execution"} -> delegated handlers + - anything else -> heartbeat
157-171: PR description vs. implementationThe PR title/description mentions “table-based branching,” but this change uses structural pattern matching, not a dispatch table. Consider updating the PR text for clarity.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
src/app/endpoints/streaming_query.py(1 hunks)
⏰ 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). (2)
- GitHub Check: build-pr
- GitHub Check: e2e_tests
🔇 Additional comments (1)
src/app/endpoints/streaming_query.py (1)
157-171: Python version requirement confirmed
pyproject.toml already declaresrequires-python = ">=3.12,<3.14", ensuring support for pattern matching (Python 3.10+) across all runtimes.
tisnik
force-pushed
the
refactoring-table-based-branching
branch
from
f9e0c6d to
8a0f54f
Compare
|
@CodeRabbit full review |
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/app/endpoints/streaming_query.py (1)
137-145: Fix typos and tighten dispatch docstring.Minor nits: fix “dispatche” and avoid “into … format” phrasing. Clarify item 1 wording.
- formats them into Server-Sent Events (SSE) format for the client. It - dispatche on (event_type, step_type): + formats them as Server-Sent Events (SSE) for the client. It + dispatches on (event_type, step_type): - 1. turn_start, turn_awaiting_input -> start token + 1. turn_start, turn_awaiting_input -> token event (turn start)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
src/app/endpoints/streaming_query.py(2 hunks)
⏰ 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). (1)
- GitHub Check: build-pr
🔇 Additional comments (1)
src/app/endpoints/streaming_query.py (1)
159-176: Include chunk_id in fallback debug; match-case is safe under Python ≥ 3.12In
src/app/endpoints/streaming_query.py, update the unhandled-event log to includechunk_id:- logger.debug( - "Unhandled event combo: event_type=%s, step_type=%s", - event_type, - step_type, - ) + logger.debug( + "Unhandled event combo id=%s: event_type=%r, step_type=%r", + chunk_id, + event_type, + step_type, + )
pyproject.tomlalready declaresrequires-python = ">=3.12,<3.14", so thematchstatement is supported.
tisnik
force-pushed
the
refactoring-table-based-branching
branch
from
8a0f54f to
facf446
Compare
1 participant
Description
Refactoring: table based branching is a bit better that if-elif with different tests
Type of change
Summary by CodeRabbit