Repository navigation
Show better error message if eval uses invalid tags - #774
Conversation
|
Warning Rate limit exceeded@aantn has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 7 minutes and 49 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 (2)
WalkthroughThe update enhances error handling in the validation process for Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~7 minutes Possibly related PRs
Suggested reviewers
✨ 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 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: 1
🧹 Nitpick comments (3)
tests/llm/utils/test_case_utils.py (3)
155-155: Fix typo in error message.There's a grammatical error in the error message.
- error_msg += f"Allowed tags; {get_allowed_tags_list()}" + error_msg += f"Allowed tags: {get_allowed_tags_list()}"
148-148: Handle different input types more robustly.The
error["input"]might not always be a string tag - it could be a list, None, or other types depending on the validation error.- problematic_tags.append(error["input"]) + input_value = error["input"] + if isinstance(input_value, list): + problematic_tags.extend(str(tag) for tag in input_value) + else: + problematic_tags.append(str(input_value))This ensures the error message remains clear regardless of the input type.
138-157: Consider extracting error handling logic for better maintainability.The enhanced error handling works correctly but adds complexity to an already large method. For better maintainability and potential reuse, consider extracting this logic to a helper function.
def _handle_validation_error_with_tag_details( e: ValidationError, test_case_folder: Path ) -> None: """Handle ValidationError and print detailed tag error information if applicable.""" problematic_tags = [] for error in e.errors(): if error["type"] == "literal_error" and len(error["loc"]) > 0 and error["loc"][-1] == "tags": input_value = error["input"] if isinstance(input_value, list): problematic_tags.extend(str(tag) for tag in input_value) else: problematic_tags.append(str(input_value)) if problematic_tags: error_msg = f"VALIDATION ERROR in test case: {test_case_folder.name}\n" error_msg += f"Problematic tags: {', '.join(problematic_tags)}\n" error_msg += f"Allowed tags: {get_allowed_tags_list()}" print(error_msg)Then use it in the except block:
except ValidationError as e: _handle_validation_error_with_tag_details(e, test_case_folder) raise e
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
tests/llm/utils/test_case_utils.py(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py
📄 CodeRabbit Inference Engine (CLAUDE.md)
**/*.py: ALWAYS place Python imports at the top of the file, not inside functions or methods
Use Ruff for formatting and linting (configured in pyproject.toml)
Type hints required (mypy configuration in pyproject.toml)
Files:
tests/llm/utils/test_case_utils.py
🧠 Learnings (3)
📓 Common learnings
Learnt from: nherment
PR: robusta-dev/holmesgpt#610
File: .github/workflows/llm-evaluation.yaml:39-42
Timestamp: 2025-07-08T08:45:41.069Z
Learning: The robusta-dev/holmesgpt codebase has comprehensive existing validation for Azure environment variables (AZURE_API_BASE, AZURE_API_KEY, AZURE_API_VERSION) and MODEL in tests/llm/utils/classifiers.py, tests/llm/conftest.py, and holmes/core/llm.py. Don't suggest adding redundant validation logic.
Learnt from: nherment
PR: robusta-dev/holmesgpt#610
File: .github/workflows/llm-evaluation.yaml:39-42
Timestamp: 2025-07-08T08:45:41.069Z
Learning: When suggesting improvements to environment variable handling in robusta-dev/holmesgpt, check first if validation logic already exists rather than reimplementing it.
Learnt from: nherment
PR: robusta-dev/holmesgpt#436
File: tests/llm/utils/mock_utils.py:240-249
Timestamp: 2025-06-05T12:23:27.634Z
Learning: The holmesgpt project uses Python >= 3.10 and prefers modern type hint syntax like `list[str]`, `dict[str, int]` over importing equivalent types from the typing module like `List[str]`, `Dict[str, int]`.
📚 Learning: the robusta-dev/holmesgpt codebase has comprehensive existing validation for azure environment varia...
Learnt from: nherment
PR: robusta-dev/holmesgpt#610
File: .github/workflows/llm-evaluation.yaml:39-42
Timestamp: 2025-07-08T08:45:41.069Z
Learning: The robusta-dev/holmesgpt codebase has comprehensive existing validation for Azure environment variables (AZURE_API_BASE, AZURE_API_KEY, AZURE_API_VERSION) and MODEL in tests/llm/utils/classifiers.py, tests/llm/conftest.py, and holmes/core/llm.py. Don't suggest adding redundant validation logic.
Applied to files:
tests/llm/utils/test_case_utils.py
📚 Learning: in llm-as-judge test cases for holmesgpt, expected outputs should be descriptive rather than prescri...
Learnt from: Sheeproid
PR: robusta-dev/holmesgpt#586
File: tests/llm/fixtures/test_ask_holmes/03_what_is_the_command_to_port_forward/test_case.yaml:4-4
Timestamp: 2025-07-02T10:27:17.231Z
Learning: In LLM-as-judge test cases for HolmesGPT, expected outputs should be descriptive rather than prescriptive when testing for flexible responses like port numbers. Using specific values in expected outputs can cause unnecessary test failures when the AI generates different but equally valid responses.
Applied to files:
tests/llm/utils/test_case_utils.py
⏰ 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). (3)
- GitHub Check: llm_evals
- GitHub Check: Pre-commit checks
- GitHub Check: Pre-commit checks
🔇 Additional comments (2)
tests/llm/utils/test_case_utils.py (2)
11-11: LGTM!The ValidationError import is correctly added and necessary for the enhanced error handling.
16-16: LGTM!The get_allowed_tags_list import is correctly added and used appropriately in the error handling logic.
2 participants
No description provided.