-
-
Notifications
You must be signed in to change notification settings - Fork 255
Added question detector to nestbot mentions #2473
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
Added question detector to nestbot mentions #2473
Conversation
Summary by CodeRabbit
WalkthroughThe changes add OWASP-related question detection to the AI query processor by introducing a pre-check that returns a default response for non-OWASP questions, short-circuiting agent execution. Corresponding test coverage is added for both OWASP and non-OWASP query scenarios. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes The changes follow a straightforward pre-check pattern with localized, cohesive modifications across two files. The new logic is relatively simple (question type validation), and test additions are consistent repetitions of similar scenarios. Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ 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 |
|
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: 1
🧹 Nitpick comments (2)
backend/apps/slack/common/handlers/ai.py (1)
66-73: Consider a more helpful default response message.The current message is terse and may not provide sufficient guidance to users who are unfamiliar with OWASP or unsure what constitutes an OWASP-related question.
Consider enriching the message:
def get_default_response() -> str: """Get default response for non-OWASP questions. Returns: str: A default response for non-OWASP questions. """ - return "Please ask questions related to OWASP." + return ( + "I can only answer questions related to OWASP (Open Web Application Security Project). " + "Please ask about OWASP projects, security standards, vulnerabilities, or best practices." + )backend/tests/apps/slack/common/handlers/ai_test.py (1)
137-151: Add assertion to verify AgenticRAGAgent is not invoked.The test correctly verifies the default response is returned for non-OWASP questions, but should also confirm that the expensive agent execution is short-circuited.
Apply this diff to add the verification:
+ @patch("apps.slack.common.handlers.ai.AgenticRAGAgent") @patch("apps.slack.common.handlers.ai.QuestionDetector") - def test_process_ai_query_non_owasp_question(self, mock_question_detector_class): + def test_process_ai_query_non_owasp_question( + self, mock_question_detector_class, mock_agent_class + ): """Test AI query processing when question is not OWASP-related.""" query = "What is the weather today?" mock_question_detector = Mock() mock_question_detector.is_owasp_question.return_value = False mock_question_detector_class.return_value = mock_question_detector result = process_ai_query(query) mock_question_detector_class.assert_called_once() mock_question_detector.is_owasp_question.assert_called_once_with(text=query) + mock_agent_class.assert_not_called() assert result == get_default_response()
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
backend/apps/slack/common/handlers/ai.py(3 hunks)backend/tests/apps/slack/common/handlers/ai_test.py(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
backend/tests/apps/slack/common/handlers/ai_test.py (3)
backend/apps/slack/common/handlers/ai.py (4)
get_blocks(14-29)get_default_response(66-73)get_error_blocks(51-63)process_ai_query(32-48)backend/apps/slack/common/question_detector.py (1)
is_owasp_question(42-69)backend/apps/ai/agent/agent.py (1)
run(27-52)
backend/apps/slack/common/handlers/ai.py (2)
backend/apps/slack/common/question_detector.py (2)
QuestionDetector(20-155)is_owasp_question(42-69)backend/apps/slack/models/message.py (1)
text(83-85)
⏰ 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). (4)
- GitHub Check: Run frontend unit tests
- GitHub Check: Run backend tests
- GitHub Check: Run frontend e2e tests
- GitHub Check: CodeQL (javascript-typescript)
🔇 Additional comments (3)
backend/apps/slack/common/handlers/ai.py (1)
9-9: LGTM!The import is correctly added for the new question detection functionality.
backend/tests/apps/slack/common/handlers/ai_test.py (2)
7-12: LGTM!The import correctly adds
get_default_responsewhich is needed for the new test assertions.
70-136: LGTM!The existing tests are correctly updated to mock
QuestionDetectorwithis_owasp_questionreturningTrue, ensuring these tests continue to focus on the agent execution path while properly integrating the new question detection dependency.



Proposed change
Checklist
make check-testlocally; all checks and tests passed.