Conversation
Signed-off-by: Codex <codex@openai.com>
WalkthroughThe PR introduces a new GitHub Actions workflow enabling manual HolmesGPT evaluations triggered via PR comments, updates an existing evaluation workflow to include rerun instructions in its report, and adds documentation on how to invoke evaluations from PR comments. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant GH as GitHub
participant WF as Workflow
participant Parser
participant Setup
participant Pytest
participant Report
participant PR as PR Comments
User->>GH: Comment with /run-evals command
GH->>WF: Trigger workflow on comment
WF->>WF: Check user permissions<br/>(collaborator/member/owner)
alt Unauthorized
WF->>PR: Post rejection message
else Authorized
WF->>Parser: Parse /run-evals comment
Parser->>WF: Extract test path, keywords,<br/>markers, workers, defaults
WF->>PR: Announce eval start
WF->>Setup: Setup HolmesGPT environment<br/>and KIND cluster
Setup->>WF: Environment ready
WF->>Pytest: Execute pytest with<br/>constructed command
Pytest->>Pytest: Run evaluations
Pytest->>WF: Capture logs & metrics
WF->>Report: Generate Markdown report<br/>(status, timing, command)
WF->>GH: Upload eval log artifact
Report->>PR: Post final result comment<br/>(report + invocation details)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks✅ Passed checks (3 passed)
Comment |
![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)
.github/workflows/eval-regression.yaml (1)
65-85: The backslash escape for the separator is unnecessary in Markdown context.Line 70 uses
\---with a comment about escaping to avoid YAML document start. However, this heredoc writes to a Markdown file (evals_report.md), not a YAML file. The backslash will appear literally in the output. If the intent is a horizontal rule, use---without the backslash; if it's meant to be a visual separator, consider using---or***directly.Suggested fix
cat <<'EOF' >> evals_report.md - \--- # separator (escaped to avoid YAML document start) + + --- Want to rerun evals with custom filters or models? Comment on this PR with:docs/development/evaluations/running-evals.md (2)
202-212: Add a language specifier to the fenced code block.The code block starting at line 204 lacks a language identifier, which triggers MD040. Since this is a command template, consider using
textorbashas the language.Suggested fix
**Command format (defaults in parentheses)** -``` +```text /run-evals models=<comma-separated models> \ # default: gpt-4o markers="<pytest -m expression>" \ # default: "llm and easy"
216-224: Consider using proper headings instead of bold text for section labels.Lines 202 and 216 use bold emphasis (
**Command format...**,**Examples**) as pseudo-headings. Converting these to###headings would improve document structure, accessibility, and resolve the MD036 linting warnings.Suggested fix
-**Command format (defaults in parentheses)** +### Command format (defaults in parentheses)-**Examples** +### Examples
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.github/workflows/eval-regression.yaml.github/workflows/manual-evals.yamldocs/development/evaluations/running-evals.md
🧰 Additional context used
📓 Path-based instructions (1)
docs/**/*.md
📄 CodeRabbit inference engine (CLAUDE.md)
When writing documentation in the docs/ directory, always add a blank line between headers/bold text and lists for proper MkDocs rendering
Files:
docs/development/evaluations/running-evals.md
🧠 Learnings (4)
📚 Learning: 2025-12-21T13:17:48.366Z
Learnt from: CR
Repo: HolmesGPT/holmesgpt PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-21T13:17:48.366Z
Learning: LLM evaluation tests run automatically in CI
Applied to files:
.github/workflows/eval-regression.yamldocs/development/evaluations/running-evals.md
📚 Learning: 2025-12-21T13:17:48.366Z
Learnt from: CR
Repo: HolmesGPT/holmesgpt PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-21T13:17:48.366Z
Learning: Applies to tests/llm/** : Implement full architecture even if complex in evals (e.g., use Loki for log aggregation properly, not simplified alternatives)
Applied to files:
docs/development/evaluations/running-evals.md
📚 Learning: 2025-12-21T13:17:48.366Z
Learnt from: CR
Repo: HolmesGPT/holmesgpt PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-21T13:17:48.366Z
Learning: Applies to tests/llm/** : No fake/obvious logs in eval scenarios - avoid logs like 'Memory usage stabilized at 800MB'
Applied to files:
docs/development/evaluations/running-evals.md
📚 Learning: 2025-12-21T13:17:48.366Z
Learnt from: CR
Repo: HolmesGPT/holmesgpt PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-21T13:17:48.366Z
Learning: Applies to tests/llm/**/*.yaml : Never use resource names that hint at the problem or expected behavior in evals - use neutral names that don't give away what the LLM should discover
Applied to files:
docs/development/evaluations/running-evals.md
🪛 LanguageTool
docs/development/evaluations/running-evals.md
[uncategorized] ~196-~196: The official name of this software platform is spelled with a capital “H”.
Context: ...regression slice of the eval suite (see .github/workflows/eval-regression.yaml). Use t...
(GITHUB)
🪛 markdownlint-cli2 (0.18.1)
docs/development/evaluations/running-evals.md
202-202: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
204-204: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
216-216: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
⏰ 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). (5)
- GitHub Check: llm_evals
- GitHub Check: build (3.12)
- GitHub Check: build (3.10)
- GitHub Check: build (3.11)
- GitHub Check: build
🔇 Additional comments (7)
docs/development/evaluations/running-evals.md (1)
194-233: Documentation aligns well with the workflow implementation.The documented parameters, defaults, and command format match the implementation in
.github/workflows/manual-evals.yaml. The examples are practical and demonstrate common use cases effectively..github/workflows/manual-evals.yaml (6)
3-15: Workflow trigger and job condition are correctly configured.The
issue_commenttrigger with the PR check (github.event.issue.pull_request != '') is the correct pattern for handling slash commands on pull requests. Permissions are appropriately scoped.
93-106: Authorization feedback mechanism is well-designed.The condition ensures a response is only posted when there's a meaningful message (unauthorized attempt), avoiding noise on non-trigger comments.
223-252: Report generation is comprehensive and informative.The generated report includes all relevant parameters, timing information, and a clear success/failure indicator. The download link correctly points to the workflow run for log access.
254-261: Fresh comment per manual run is correctly configured.Setting
delete-previous: 'false'ensures each manual evaluation run posts a new comment, preserving the history of manual runs as intended by the PR objectives.
62-62: Thetest_pathparameter is user-controllable but trusted.The
test_pathparameter defaults totests/llm/but could be overridden by authorized users. Since only collaborators/members/owners can trigger this workflow (checked at line 48), this is acceptable. However, note thattest_pathis not documented in the docs or workflow instructions—users may not know they can customize it.Is
test_pathintentionally undocumented to discourage its use, or should it be added to the documentation?
185-185: This hardcoding is intentional and appropriate—no refactoring needed.The
--strict-setup-exceptions=22_high_latency_dbi_downtargets a specific test case fixture (matchingtests/llm/fixtures/test_ask_holmes/22_high_latency_dbi_down/test_case.yaml), not an environment-specific setting. The value is documented in evaluation results and is not intended to change. Parameterizing this would add unnecessary complexity without benefit.
| set_output("response-message", response_message) | ||
| exit(0) | ||
|
|
||
| tokens = shlex.split(body) |
There was a problem hiding this comment.
Wrap shlex.split() in a try-except to handle malformed input gracefully.
If a user posts a comment with unclosed quotes (e.g., /run-evals models="gpt-4o), shlex.split() will raise a ValueError, causing the step to fail with an unclear error. Consider catching this and posting a helpful response.
Suggested fix
- tokens = shlex.split(body)
+ try:
+ tokens = shlex.split(body)
+ except ValueError as e:
+ response_message = f"⚠️ Could not parse command: {e}. Check for unclosed quotes."
+ set_output("should-run", "false")
+ set_output("response-message", response_message)
+ exit(0)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| tokens = shlex.split(body) | |
| try: | |
| tokens = shlex.split(body) | |
| except ValueError as e: | |
| response_message = f"⚠️ Could not parse command: {e}. Check for unclosed quotes." | |
| set_output("should-run", "false") | |
| set_output("response-message", response_message) | |
| exit(0) |
🤖 Prompt for AI Agents
In .github/workflows/manual-evals.yaml around line 56, the call tokens =
shlex.split(body) can raise ValueError for malformed input (e.g., unclosed
quotes); wrap this call in a try/except that catches ValueError, handle it by
posting a clear user-facing response (or logging) explaining the parse error and
how to format the command, and ensure the workflow step exits gracefully (do not
let the exception propagate and fail the job).
Results of HolmesGPT evals
Legend
Want to rerun evals with custom filters or models? Comment on this PR with: Defaults: models=gpt-4o, markers="llm and easy", keyword="", iterations=1, workers=6, classifier_model=gpt-4o. Examples:
See docs/development/evaluations/running-evals.md for details. |
|
Dev Docker images are ready for this commit:
Use either tag to pull the image for testing. |
|
Dev Docker images are ready for this commit:
Use either tag to pull the image for testing. |
|
/run-evals keyword=80_pvc_storage_class_mismatch iterations=2 |
|
/run-evals models=gpt-4o markers="llm and easy" keyword="" |
1 participant
Summary
Testing
Codex Task
Summary by CodeRabbit
New Features
Improvements
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.