fix: Handle None responses in ask function and update prepare_answer_… #121
Conversation
…sources to filter out None nodes
WalkthroughThe pull request introduces two key modifications: enhanced error handling in the Changes
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 (
|
![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 (4)
routers/amqp.py (1)
64-66: Enhance error handling with more context.While checking for None responses is good, the error message could be more descriptive. Consider including the community_id and question in the error message to aid debugging.
- raise ValueError("not confident in answering!") + raise ValueError(f"Not confident in answering question for community {community_id}. Question: {question}")utils/query_engine/prepare_answer_sources.py (3)
73-74: Add logging for filtered None nodes.Consider logging when None nodes are filtered out to help with debugging and monitoring.
+ none_count = sum(1 for n in nodes if n is None) + if none_count > 0: + logging.warning(f"Filtered out {none_count} None nodes from input") cleaned_nodes = [n for n in nodes if n is not None]
24-24: Consider early input validation.The method accepts potentially None nodes but only filters them after the empty check. Consider validating and cleaning the input earlier to maintain consistent behavior.
- def prepare_answer_sources(self, nodes: list[NodeWithScore | None]) -> str: - if len(nodes) == 0: + def prepare_answer_sources(self, nodes: list[NodeWithScore | None]) -> str: + if not nodes: + logging.error("No reference nodes available! returning empty string.") + return "" + + cleaned_nodes = [n for n in nodes if n is not None] + if not cleaned_nodes: logging.error("No reference nodes available! returning empty string.") return "" - - cleaned_nodes = [n for n in nodes if n is not None]Also applies to: 73-77
73-74: Consider using filter() for better readability.The list comprehension could be replaced with filter() for a more functional approach.
- cleaned_nodes = [n for n in nodes if n is not None] + cleaned_nodes = list(filter(None, nodes))
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
routers/amqp.py(1 hunks)utils/query_engine/prepare_answer_sources.py(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: ci / lint / Lint
- GitHub Check: ci / test / Test
🔇 Additional comments (1)
routers/amqp.py (1)
64-66: Verify error handling in consumer.The ValueError will cause the message to be rejected. Ensure the consumer properly handles this specific error case, possibly by implementing a dead letter queue (DLQ) for failed messages.
✅ Verification successful
Error handling is properly implemented ✅
The ValueError is handled appropriately as it's caught by the global try-except block in the AMQP consumer. The code uses FastStream's RabbitRouter which automatically handles message acknowledgments and rejections. Failed messages are logged with proper context (community_id) for monitoring and debugging purposes.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for error handling or DLQ configuration in consumer code rg -A 5 "dead.*letter|dlq|error.*queue"Length of output: 41
Script:
#!/bin/bash # Search for AMQP configuration and message handling patterns echo "=== AMQP Configuration ===" rg -l "import.*pika|rabbitmq|amqp" echo -e "\n=== Message Processing and Error Handling ===" ast-grep --pattern 'try { $$$ } except $_: { $$$ }' echo -e "\n=== Message Acknowledgment Patterns ===" rg -B2 -A2 "basic_ack|basic_nack|basic_reject"Length of output: 647
Script:
#!/bin/bash # Examine AMQP router implementation echo "=== AMQP Router Content ===" cat routers/amqp.py echo -e "\n=== Python Error Handling Patterns ===" ast-grep --pattern 'try: $$$ except $_: $$$' echo -e "\n=== Message Handling Patterns ===" rg -B3 -A3 "channel\.|message\.|delivery"Length of output: 11715
…sources to filter out None nodes
Summary by CodeRabbit
Bug Fixes
Refactor