Skip to content

Allow manually triggering evals - #1258

Merged
aantn merged 16 commits into
masterfrom
claude/manual-eval-trigger-CZGZL
Dec 29, 2025
Merged

aantn merged 16 commits into
masterfrom
claude/manual-eval-trigger-CZGZL

Conversation

@aantn

@aantn aantn commented Dec 29, 2025 •

Copy link
Copy Markdown
Collaborator

Summary by CodeRabbit

  • New Features
    • Manually trigger evaluations from the workflow UI or with a /eval PR comment, including a re-run UI in results.
  • Chores
    • Pass configurable parameters (model, markers, filter, iterations) to runs and build dynamic test invocations.
    • Provide status comments, reactions and clearer results reporting for manual and automatic runs.
  • Bug Fixes
    • Ensure correct revision is checked out for comment-triggered evaluations.

✏️ Tip: You can customize this high-level summary in your review settings.

claude and others added 13 commits December 28, 2025 10:44
Features:
- Add /eval command support in PR comments with options:
  --model/-m: Specify model(s) to test
  --markers/-M: Pytest markers (default: regression)
  --filter/-k: Pytest -k filter
  --iterations/-i: Number of iterations (max 10)
- Add workflow_dispatch trigger for GitHub UI triggering
- Post initial "running" comment when evals start
- Update same comment with results when complete
- Each run preserves its own comment (history maintained)
- Input validation to prevent shell injection
- Add manual trigger instructions in automatic run comments
- Consolidate PR number logic into eval-params step
- Remove duplicate validation (parse step extracts, eval-params validates)
- Add run_url output to avoid recalculating
- Use shared params object in comment steps
- Condense duration formatting in bash
- 621 → 387 lines
Before:
  /eval --model gpt-4o --filter "my test"

After:
  /eval
  model: gpt-4o
  filter: my test

Much simpler parsing (split on newlines, split on colon) and
easier for humans to read/write in PR comments.
- Simplify parse-eval-command job to just: get PR SHA + add reaction
- Move parseComment() function into eval-params step
- Remove 4 outputs (model, markers, filter, iterations) from job 1
- Eliminates duplicate passing of values between jobs
- 388 → 357 lines
- Consolidate parse-eval-command job into llm_evals as a step
- Add permission check: only OWNER, MEMBER, COLLABORATOR can trigger /eval
- Prevents random external users from triggering expensive evals
- Simplifies workflow structure (1 job instead of 2)
Signed-off-by: Natan Yellin <aantny@gmail.com>
- Fix workflow dispatch link to point to specific workflow file
- Change rocket reaction to eyes for clearer "processing" signal
- Simplify secrets check (repo has all or none)

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Signed-off-by: Natan Yellin <aantny@gmail.com>
…/HolmesGPT/holmesgpt into claude/manual-eval-trigger-CZGZL

Signed-off-by: Natan Yellin <aantny@gmail.com>
@coderabbitai

coderabbitai Bot commented Dec 29, 2025 •

Copy link
Copy Markdown
Contributor

Walkthrough

Adds manual and comment-triggered eval runs to the existing eval-regression workflow: supports workflow_dispatch inputs, /eval PR comments with permission checks, dynamic parameter parsing/validation, conditional checkout of PR head SHA, parameterized pytest execution, and enhanced PR result comments and reactions.

Changes

Cohort / File(s) Summary
Workflow triggers & permissions
.github/workflows/eval-regression.yaml
Adds workflow_dispatch inputs (pr_number, model, markers, filter, iterations) and issue_comment trigger for /eval; expands permissions to include issues: write; multi-branch job condition for PRs, pushes, manual dispatch, or /eval.
Comment handling & checkout
.github/workflows/eval-regression.yaml
New step to validate /eval commenter permissions (OWNER/MEMBER/COLLABORATOR), add reaction, and fetch PR head SHA; checkout adjusted to use PR SHA when present.
Parameter parsing & validation
.github/workflows/eval-regression.yaml
New "Determine eval parameters" script parses inputs from comment or workflow_dispatch, validates against whitelisted regexes, applies defaults, and emits outputs (model, markers, filter, iterations, is_manual, trigger_source, pr_number, marker_expr, run_url).
Secrets & conditional run gating
.github/workflows/eval-regression.yaml
Simplified secret-check gating based on presence of AZURE_API_KEY; conditionally sets should-run.
Test execution
.github/workflows/eval-regression.yaml
Reworks "Run tests" to pass MODEL, ITERATIONS, EVAL_MARKER_EXPR, EVAL_FILTER to the runner; dynamically builds pytest args (markers and optional -k filter) and outputs human-friendly duration.
Environment & infra setup
.github/workflows/eval-regression.yaml
Adds conditional steps to set up KIND cluster and HolmesGPT environment when tests should run.
Result reporting & PR comments
.github/workflows/eval-regression.yaml
Replaces static post-commenting with conditional, parameterized results comment reading optional report/regressions files; formats manual vs automatic runs differently; includes re-run-evals UI block and updates/creates PR comment; adds completion reaction for comment-triggered runs; preserves "Check test results" step to surface regressions.

Sequence Diagram(s)

sequenceDiagram
  participant User as GitHub User
  participant GH as GitHub (Events)
  participant Actions as GitHub Actions Runner
  participant Repo as Repository Checkout
  participant Test as Test Runner / pytest
  participant PR as Pull Request (comments)

  rect rgba(200,230,255,0.3)
    User->>GH: /eval comment on PR (or triggers workflow_dispatch)
    GH->>Actions: dispatch workflow (issue_comment or workflow_dispatch)
  end

  rect rgba(230,255,200,0.25)
    Actions->>GH: validate commenter permissions (if issue_comment)
    GH-->>Actions: permission result
    Actions->>GH: fetch PR head SHA (if comment)
    GH-->>Actions: PR head SHA
    Actions->>Repo: checkout appropriate ref (PR SHA or ref)
    Repo-->>Actions: code checked out
  end

  rect rgba(255,240,200,0.25)
    Actions->>Actions: Determine eval parameters (parse inputs, validate regex, set defaults)
    Actions->>Actions: check AZURE_API_KEY -> decide should-run
    Actions->>Actions: setup KIND & environment (conditional)
  end

  rect rgba(255,220,220,0.15)
    Actions->>Test: run pytest with dynamic args (MODEL, ITERATIONS, marker expr, filter)
    Test-->>Actions: test results, optional report/regressions
    Actions->>GH: post or update PR comment with results (manual vs automatic formatting)
    Actions->>GH: add reactions (eyes, completion) for comment-triggered runs
  end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Suggested reviewers

  • moshemorad

Pre-merge checks

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main purpose of the pull request - adding manual trigger functionality for evaluations. It directly corresponds to the primary change: introducing workflow_dispatch and issue_comment triggers to enable on-demand evals.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

📜 Recent review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 955816d and f307351.

📒 Files selected for processing (1)
  • .github/workflows/eval-regression.yaml
⏰ 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: build
  • GitHub Check: build (3.10)
  • GitHub Check: build (3.12)
  • GitHub Check: build (3.11)
  • GitHub Check: llm_evals
🔇 Additional comments (12)
.github/workflows/eval-regression.yaml (12)

8-31: LGTM! Well-designed workflow inputs.

The workflow_dispatch inputs are comprehensive with clear descriptions and examples. The issue_comment trigger is appropriately scoped to created events only.


36-36: LGTM! Permission appropriately scoped.

The issues: write permission is necessary for posting status comments and reactions on PRs.


41-45: LGTM! Conditional logic is correct.

The multi-branch condition properly gates the job execution on valid trigger events and ensures /eval comments are only processed on pull requests.


48-76: LGTM! Good security and UX practices.

The permission check appropriately restricts eval triggering to trusted users, and fetching the PR HEAD SHA ensures the correct code revision is evaluated. The eyes reaction provides helpful feedback to users.


78-81: LGTM! Secure checkout strategy.

Conditionally checking out the PR HEAD SHA (when available) prevents potential security issues where malicious code could be evaluated without proper review.


82-169: LGTM! Excellent security practices.

The parameter validation and parsing logic demonstrates strong security awareness:

  • Whitelist regex patterns prevent injection attacks
  • Proper use of toJSON() for GitHub context values
  • Clear security comment explaining the defense strategy
  • Appropriate clamping of iterations to prevent resource exhaustion
  • Safe construction of the marker expression

The validation patterns are appropriate for their respective use cases (model names with provider prefixes, pytest markers, test name filters).


173-181: LGTM! Simplified secret check.

Using a single secret as a proxy for secrets access is a reasonable approach, especially when secrets are typically configured as a set.


183-219: LGTM! Good user experience.

Posting an initial status comment provides immediate feedback to users, and storing the comment ID enables updating the same comment with results later. The different formatting for manual vs automatic runs helps users understand the context.


246-256: LGTM! Parameterized environment variables.

The dynamic environment variables correctly use the validated outputs from the eval-params step. Note that this introduces a dependency on the ANTHROPIC_API_KEY secret, which aligns with the Anthropic model support mentioned in the workflow_dispatch input descriptions.


257-272: LGTM! Safe parameterized test execution.

The pytest invocation correctly uses environment variables with proper quoting, preventing shell injection. The dynamic argument construction and human-friendly duration formatting are well-implemented. The || true appropriately allows the workflow to continue processing results even when tests fail.


273-336: LGTM! Comprehensive results reporting.

The results posting logic handles both manual and automatic runs appropriately, with clear formatting and actionable re-run instructions. The conditional update or create logic ensures the right comment is modified when available.


337-346: LGTM! Nice UX touch.

Adding a completion reaction provides clear visual feedback that the eval has finished processing, complementing the detailed results comment.


Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions

github-actions Bot commented Dec 29, 2025 •

Copy link
Copy Markdown
Contributor

✅ Docker image ready for c1ea6b1 (built in 37s)

⚠️ Warning: does not support ARM (ARM images are built on release only - not on every PR)

Use this tag to pull the image for testing.

📋 Copy commands

⚠️ Temporary images are deleted after 30 days. Copy to a permanent registry before using them:

gcloud auth configure-docker us-central1-docker.pkg.dev
docker pull us-central1-docker.pkg.dev/robusta-development/temporary-builds/holmes:c1ea6b1
docker tag us-central1-docker.pkg.dev/robusta-development/temporary-builds/holmes:c1ea6b1 me-west1-docker.pkg.dev/robusta-development/development/holmes-dev:c1ea6b1
docker push me-west1-docker.pkg.dev/robusta-development/development/holmes-dev:c1ea6b1

Patch Helm values in one line (choose the chart you use):

HolmesGPT chart:

helm upgrade --install holmesgpt ./helm/holmes \
  --set registry=me-west1-docker.pkg.dev/robusta-development/development \
  --set image=holmes-dev:c1ea6b1

Robusta wrapper chart:

helm upgrade --install robusta robusta/robusta \
  --reuse-values \
  --set holmes.registry=me-west1-docker.pkg.dev/robusta-development/development \
  --set holmes.image=holmes-dev:c1ea6b1

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

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 (1)
.github/workflows/eval-regression.yaml (1)

171-181: Secrets check is functional but could use env var indirection.

The pattern works and secrets are masked, but passing via env: would be slightly safer against edge-case log leaks.

🔎 Optional improvement using env var
      - name: Check if tests should run
        id: check-tests
        if: github.event_name != 'issue_comment' || steps.eval-comment.outcome == 'success'
        shell: bash
+       env:
+         HAS_SECRETS: ${{ secrets.AZURE_API_KEY != '' }}
        run: |
-         # Check one secret as proxy for secrets access (repo has all or none)
-         if [[ -n "${{ secrets.AZURE_API_KEY }}" ]]; then
+         if [[ "$HAS_SECRETS" == "true" ]]; then
            echo "should-run=true" >> $GITHUB_OUTPUT
          else
            echo "should-run=false" >> $GITHUB_OUTPUT
          fi
📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c35e334 and 955816d.

📒 Files selected for processing (1)
  • .github/workflows/eval-regression.yaml
🧰 Additional context used
🪛 actionlint (1.7.9)
.github/workflows/eval-regression.yaml

87-87: unexpected end of input while parsing variable access, function call, null, bool, int, float or string. expecting "IDENT", "(", "INTEGER", "FLOAT", "STRING"

(expression)

⏰ 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: build (3.12)
  • GitHub Check: build (3.11)
  • GitHub Check: build (3.10)
  • GitHub Check: build
🔇 Additional comments (12)
.github/workflows/eval-regression.yaml (12)

8-31: Well-structured workflow_dispatch and issue_comment triggers.

The input definitions with clear descriptions and sensible defaults are good for usability. The issue_comment trigger scoped to created type is appropriate.


33-36: Permissions are appropriately scoped.

The added issues: write permission is required for the reactions and comment functionality on issue_comment triggers.


41-45: Job condition correctly gates the workflow.

The multi-condition check properly ensures that issue_comment events only trigger for PR comments that start with /eval.


48-75: Good permission validation and user feedback.

The authorization check via author_association is a solid security practice. The 'eyes' reaction provides immediate feedback to the user that their comment was recognized.


77-80: Checkout logic handles all trigger types correctly.

The conditional checkout with fallback from PR SHA to github.ref appropriately handles both comment-triggered and other event types.


82-169: Well-designed input validation with whitelist patterns.

The security approach is solid: whitelist regex patterns combined with toJSON() for proper escaping. The parameter parsing for /eval comments is cleanly implemented.

The static analysis warning on line 87 is a false positive — toJSON() expressions are valid within actions/github-script blocks and produce properly quoted JavaScript string literals.


183-218: Initial comment provides good visibility into running evals.

The conditional formatting for manual vs automatic runs and the use of a markdown table for parameters provides clear status reporting.


220-232: Setup steps properly gated by secrets availability check.


234-271: Secure test execution with properly quoted variables.

The test step correctly follows the security guidance: user inputs are passed via environment variables and properly quoted in bash. The duration formatting provides useful human-readable output.


273-335: Results posting handles all cases gracefully.

The conditional update vs create logic is correct, and the detailed re-run instructions in automatic runs are helpful. The fs.existsSync checks prevent errors when report files are missing.


337-345: Completion reaction provides clear feedback.

The 'hooray' reaction gives users visible confirmation that the eval completed.


347-355: Regressions are reported but don't fail the workflow.

This step logs regressions without causing a workflow failure. If regressions should block PRs, you may want to add exit 1 when regressions are detected.

Is the current behavior (report-only, no failure) intentional? If regressions should block merging, consider:

          if [[ -f "regressions.txt" ]]; then
            echo "⚠️ There are regressions in the evals. Please check the evals file for details."
            cat regressions.txt
+           exit 1
          else
            echo "✅ All tests passed without regressions."
          fi

Remove `${{ }}` from JS comment - GitHub parses expressions even in comments.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Signed-off-by: Natan Yellin <aantny@gmail.com>
@github-actions

github-actions Bot commented Dec 29, 2025 •

Copy link
Copy Markdown
Contributor

Results of HolmesGPT evals

Duration: 4m 5s | View workflow logs

Results of HolmesGPT evals

  • ask_holmes: 7/7 test cases were successful, 0 regressions
Test suite Test case Status
ask 09_crashpod ✅
ask 101_loki_historical_logs_pod_deleted ✅
ask 12_job_crashing ✅
ask 162_get_runbooks ✅
ask 176_network_policy_blocking_traffic_no_runbooks ✅
ask 43_current_datetime_from_prompt ✅
ask 61_exact_match_counting ✅

Legend

  • ✅ the test was successful
  • :minus: the test was skipped
  • ⚠️ the test failed but is known to be flaky or known to fail
  • 🚧 the test had a setup failure (not a code regression)
  • 🔧 the test failed due to mock data issues (not a code regression)
  • 🚫 the test was throttled by API rate limits/overload
  • ❌ the test failed and should be fixed before merging the PR

🔄 Re-run evals manually

Option 1: Comment on this PR with /eval:

/eval

Or with options (one per line):

/eval
model: gpt-4o
filter: 09_crashpod
iterations: 5
Option Description
model Model(s) to test (default: same as automatic runs)
markers Pytest markers (default: regression)
filter Pytest -k filter
iterations Number of runs, max 10

Option 2: Trigger via GitHub Actions UI → "Run workflow"

@aantn
aantn enabled auto-merge (squash) December 29, 2025 11:18
@github-actions

github-actions Bot commented Dec 29, 2025 •

Copy link
Copy Markdown
Contributor

Results of HolmesGPT evals

Duration: 4m 3s | View workflow logs

Results of HolmesGPT evals

  • ask_holmes: 7/7 test cases were successful, 0 regressions
Test suite Test case Status
ask 09_crashpod ✅
ask 101_loki_historical_logs_pod_deleted ✅
ask 12_job_crashing ✅
ask 162_get_runbooks ✅
ask 176_network_policy_blocking_traffic_no_runbooks ✅
ask 43_current_datetime_from_prompt ✅
ask 61_exact_match_counting ✅

Legend

  • ✅ the test was successful
  • :minus: the test was skipped
  • ⚠️ the test failed but is known to be flaky or known to fail
  • 🚧 the test had a setup failure (not a code regression)
  • 🔧 the test failed due to mock data issues (not a code regression)
  • 🚫 the test was throttled by API rate limits/overload
  • ❌ the test failed and should be fixed before merging the PR

🔄 Re-run evals manually

Option 1: Comment on this PR with /eval:

/eval

Or with options (one per line):

/eval
model: gpt-4o
filter: 09_crashpod
iterations: 5
Option Description
model Model(s) to test (default: same as automatic runs)
markers Pytest markers (default: regression)
filter Pytest -k filter
iterations Number of runs, max 10

Option 2: Trigger via GitHub Actions UI → "Run workflow"

@github-actions

github-actions Bot commented Dec 29, 2025 •

Copy link
Copy Markdown
Contributor

Results of HolmesGPT evals

Duration: 3m 58s | View workflow logs

Results of HolmesGPT evals

  • ask_holmes: 9/9 test cases were successful, 0 regressions
Test suite Test case Status
ask 09_crashpod ✅
ask 101_loki_historical_logs_pod_deleted ✅
ask 111_pod_names_contain_service ✅
ask 12_job_crashing ✅
ask 162_get_runbooks ✅
ask 176_network_policy_blocking_traffic_no_runbooks ✅
ask 24_misconfigured_pvc ✅
ask 43_current_datetime_from_prompt ✅
ask 61_exact_match_counting ✅

Legend

  • ✅ the test was successful
  • :minus: the test was skipped
  • ⚠️ the test failed but is known to be flaky or known to fail
  • 🚧 the test had a setup failure (not a code regression)
  • 🔧 the test failed due to mock data issues (not a code regression)
  • 🚫 the test was throttled by API rate limits/overload
  • ❌ the test failed and should be fixed before merging the PR

🔄 Re-run evals manually

Option 1: Comment on this PR with /eval:

/eval

Or with options (one per line):

/eval
model: gpt-4o
filter: 09_crashpod
iterations: 5
Option Description
model Model(s) to test (default: same as automatic runs)
markers Pytest markers (default: regression)
filter Pytest -k filter
iterations Number of runs, max 10

Option 2: Trigger via GitHub Actions UI → "Run workflow"

@aantn
aantn merged commit e6ae36b into master Dec 29, 2025
10 of 11 checks passed
@aantn
aantn deleted the claude/manual-eval-trigger-CZGZL branch December 29, 2025 12:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants