-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: overriding the llama-index function to improve error handling! #125
Conversation
WalkthroughThe changes modify the Changes
Sequence DiagramsequenceDiagram
participant QE as Query Engine
participant SQE as SubQuestion Engine
participant SQ as SubQuestion
participant LG as Logger
QE->>SQE: Generate sub-questions
SQE->>SQ: Create SubQuestion
SQE->>SQE: _query_subq method
SQE->>LG: Log query process
SQE-->>QE: SubQuestionAnswerPair
Poem
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (3)
utils/query_engine/subquestion_engine.py (3)
Line range hint
39-91
: LGTM! Robust error handling for sub-question processing.The changes effectively handle failed sub-questions and maintain consistency between async and sync execution paths. The filtering of failed results before synthesis prevents cascading errors.
Consider adding a warning log when all sub-questions fail, as this might indicate a systemic issue:
qa_pairs: List[SubQuestionAnswerPair] = list(filter(None, qa_pairs_all)) if qa_pairs: nodes = [self._construct_node(pair) for pair in qa_pairs] source_nodes = [ node for qa_pair in qa_pairs for node in qa_pair.sources ] response = self._response_synthesizer.synthesize( query=query_bundle, nodes=nodes, additional_source_nodes=source_nodes, ) else: + logger.warning( + f"All {len(sub_questions)} sub-questions failed for query: {query_bundle.query_str}" + ) response = Response(response=None, source_nodes=[])
101-103
: Add return type hint for better code maintainability.The method's return type should be explicitly defined in the signature.
def _query_subq( self, sub_q: SubQuestion, color: Optional[str] = None - ) -> Optional[SubQuestionAnswerPair]: + ) -> Optional[SubQuestionAnswerPair]: # type: SubQuestionAnswerPair | None
128-132
: Enhance error logging for better debugging.The current error logging could be more informative by including the exception traceback.
- logger.warning( + logger.warning( f"[{sub_q.tool_name}] Failed to run {sub_q.sub_question}: {exp}" - ) + exc_info=True + )
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
utils/query_engine/subquestion_engine.py
(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: ci / lint / Lint
- GitHub Check: ci / test / Test
🔇 Additional comments (2)
utils/query_engine/subquestion_engine.py (2)
1-1
: LGTM! Well-organized imports and proper logger setup.The addition of logging capabilities and necessary imports enhances the module's error handling capabilities.
Also applies to: 18-18
104-127
: LGTM! Well-structured error handling with proper event tracking.The implementation effectively:
- Encapsulates sub-question processing logic
- Uses appropriate try-except pattern
- Maintains traceability through callback events
- Provides helpful debug information through verbose logging
Summary by CodeRabbit
New Features
Refactor