fix(#267): emit evaluation_complete event in full_techniques mode#269
fix(#267): emit evaluation_complete event in full_techniques mode#269
Conversation
- Import event_channel and create_sommelier_event in finalize.py - Emit quality_gate_complete event with final scores - Emit evaluation_complete event for frontend completion detection Closes #267
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Summary of ChangesHello @ComBba, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses a critical issue where the frontend was unable to detect the completion of evaluations in Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
|
Warning Rate limit exceeded
⌛ 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. Walkthrough이 변경은 Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request aims to fix a missing evaluation_complete event in the full_techniques mode by adding event emission logic in the finalize node. However, it introduces a critical regression: the quality_gate_complete event emission calls create_sommelier_event with unsupported arguments (total_score and quality_gate), leading to a TypeError and a crash in the evaluation process. To resolve this, the SSEEvent model should be used for flexible data payloads, preventing the application from crashing and ensuring evaluations complete successfully.
| from app.criteria.bmad_items import list_items, get_category, get_category_max | ||
| from app.constants import get_quality_gate | ||
| from app.models.graph import ItemScore | ||
| from app.services.event_channel import create_sommelier_event, get_event_channel |
There was a problem hiding this comment.
To correctly emit the quality_gate_complete event with a custom payload, it's necessary to use the SSEEvent model, which supports a flexible data field. The current import is missing SSEEvent and EventType, which are required for the fix. Please add them to the import statement.
| from app.services.event_channel import create_sommelier_event, get_event_channel | |
| from app.services.event_channel import SSEEvent, EventType, create_sommelier_event, get_event_channel |
| total_score=round(normalized, 2), | ||
| quality_gate=quality_gate, |
There was a problem hiding this comment.
The finalize node incorrectly calls create_sommelier_event with unsupported keyword arguments total_score and quality_gate. This mismatch will cause a TypeError at runtime, leading to a crash of the evaluation process and preventing evaluations from completing successfully in full_techniques mode. To resolve this, consider using SSEEvent with a flexible data dictionary to correctly emit the final scores and prevent application crashes.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@backend/app/graph/nodes/technique_categories/finalize.py`:
- Around line 59-70: The call to create_sommelier_event in finalize.py passes
non-existent kwargs total_score and quality_gate causing a TypeError; remove
those kwargs from event_channel.emit_sync and instead include the quality_gate
and rounded score inside the message string (e.g., append ", quality_gate:
{quality_gate}, total_score: {round(normalized,2)}") when calling
create_sommelier_event; alternatively, if you prefer schema change, add fields
to SommelierProgressEvent and update create_sommelier_event signature to accept
total_score and quality_gate across the codebase, but do not pass unknown kwargs
from finalize.py without updating the event model and factory.
🧹 Nitpick comments (1)
backend/app/graph/nodes/technique_categories/finalize.py (1)
57-80: 이벤트 방출 실패 시 에러 전파로finalize노드 전체가 실패할 수 있음이벤트 방출은 부가 기능(side-effect)이지만, 현재
try/except없이 호출되고 있어emit_sync나create_sommelier_event에서 예외가 발생하면 평가 결과(return블록)가 반환되지 않습니다. 이벤트 방출 실패가 평가 결과 손실로 이어지지 않도록 방어적 처리를 권장합니다.🛡️ try/except로 이벤트 방출을 감싸는 수정안
if evaluation_id: + try: event_channel = get_event_channel() event_channel.emit_sync( evaluation_id, create_sommelier_event( evaluation_id=evaluation_id, sommelier="finalize", event_type="quality_gate_complete", progress_percent=95, message=f"Quality gate: {quality_gate}, score: {round(normalized, 2)}", ), ) event_channel.emit_sync( evaluation_id, create_sommelier_event( evaluation_id=evaluation_id, sommelier="system", event_type="evaluation_complete", progress_percent=100, message="Evaluation complete!", ), ) + except Exception: + import logging + logging.getLogger(__name__).warning( + "Failed to emit finalize events for %s", evaluation_id, exc_info=True + )
Address review feedback: - Remove total_score and quality_gate kwargs from create_sommelier_event - Include score in message string instead - Wrap event emission in try/except to prevent node failure
…te-event fix(#267): emit evaluation_complete event in full_techniques mode
1 participant
Summary
Resolves #267
Problem
In
full_techniquesevaluation mode, theevaluation_completeSSE event was never emitted, causing the frontend to never reliably detect evaluation completion. The finalize node was missing event emission logic.Changes
create_sommelier_eventandget_event_channelinfinalize.pyquality_gate_completeevent with final scores (total_score, quality_gate)evaluation_completeevent for frontend completion detectionTesting
Checklist
Summary by CodeRabbit
새로운 기능