Repository navigation
Add test case for Holmes Elasticsearch log data transparency - #1858
Conversation
Test 254 verifies Holmes recognizes when it has no log data for the specific service (companyopswebjob) the customer asks about. ES only contains logs for Company.Ops and Company.Ops.Radar.WebJob — both have job processing failures that act as red herrings. Holmes should avoid diagnosing based on these unrelated services. Currently fails (red) because Holmes attributes Company.Ops errors to companyopswebjob without noticing the service name mismatch. https://claude.ai/code/session_018QqVoFkVGpSkSAK4SzgzEN Signed-off-by: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review to trigger a review and subscribe this PR to future pushes, or @claude review once for a one-time review.
Tip: disable this comment in your organization's Code Review settings.
📂 Previous Runs📜 #1 · Run @ __898c062__ (#23737799138) — Mar 30, 09:38 UTC✅ Results of HolmesGPT evalsAutomatically triggered by commit 898c062 on branch Results of HolmesGPT evals
Benchmark Comparison DetailsBaseline: latest ci-benchmark experiment on master Status: Success - 114 test/model combinations loaded Benchmark experiment:
No benchmark data available for comparison. Benchmark has no cost, total tokens, cached tokens data. Will appear after the next weekly benchmark run. Comparison indicators:
|
| Status | Test case | Time | Turns | Tools | Cost | Total tokens | Input | Max input | Output | Max output | Cached | Non-cached | Reasoning | Compactions |
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| ❌ | 254_elasticsearch_dr_test_log_check | 61.0s | 10 | 16 | $0.3254 | 175,963 | 171,987 | 22,895 | 3,976 | 686 | 147,239 | 24,748 | — | — |
| Total | 61.0s avg | 10.0 avg | 16.0 avg | $0.3254 | 175,963 | 171,987 | 22,895 | 3,976 | 686 | 147,239 | 24,748 | — | — |
Benchmark Comparison Details
Baseline: latest ci-benchmark experiment on master
Status: Success - 114 test/model combinations loaded
Benchmark experiment:
- ci-benchmark-23700167817 (created: 2026-03-29)
No benchmark data available for comparison.
Benchmark has no cost, total tokens, cached tokens data. Will appear after the next weekly benchmark run.
Comparison indicators:
±0%— diff under 10% (within noise threshold)↑N%/↓N%— diff 10-25%↑N%/↓N%— diff over 25% (significant)
⚠️ 1 Failure Detected
📖 Legend
| Icon | Meaning |
|---|---|
| ✅ | The test was successful |
| ➖ | 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
⚠️ Warning:/evalcomments always run using the workflow from master, not from this PR branch. If you modified the GitHub Action (e.g., added secrets or env vars), those changes won't take effect.To test workflow changes, use the GitHub CLI or Actions UI instead:
gh workflow run eval-regression.yaml --repo HolmesGPT/holmesgpt --ref claude/add-dr-test-log-check-oAq7y -f markers=regression -f filter=
Option 1: Comment on this PR with /eval:
/eval
tags: regression
Or with more options (one per line):
/eval
model: gpt-4o
tags: regression
id: 09_crashpod
iterations: 5
Run evals on a different branch (e.g., master) for comparison:
/eval
branch: master
tags: regression
| Option | Description |
|---|---|
model |
Model(s) to test (default: same as automatic runs) |
tags |
Pytest tags / markers (no default - runs all tests!) |
id |
Eval ID / pytest -k filter (use /list to see valid eval names) |
iterations |
Number of runs, max 10 |
branch |
Run evals on a different branch (for cross-branch comparison) |
Quick re-run: Use /rerun to re-run the most recent /eval on this PR with the same parameters.
Option 2: Trigger via GitHub Actions UI → "Run workflow"
Option 3: Add PR labels to include extra evals (applies to both automatic runs and /eval comments):
| Label | Effect |
|---|---|
evals-tag-<name> |
Run tests with tag <name> alongside regression |
evals-id-<name> |
Run a specific eval by test ID |
evals-model-<name> |
Override the model (use model list name, e.g. sonnet-4.5) |
Examples: evals-tag-easy, evals-id-09_crashpod, evals-model-sonnet-4.5
🏷️ Valid tags
benchmark, chain-of-causation, compaction, confluence, context_window, coralogix, counting, database, datadog, datetime, db-connectors, easy, elasticsearch, embeds, fast, frontend, grafana, hard, images, integration, kafka, kubernetes, leaked-information, logs, loki, mcp, medium, metrics, network, newrelic, no-cicd, numerical, one-test, port-forward, prometheus, question-answer, regression, runbooks, slackbot, storage, toolset-limitation, traces, transparency
🤖 Valid models
deepseek-chat, deepseek-r1-reasoner, deepseek-reasoner, deepseek-v3.2-chat, gemini-3-flash-preview, gemini-3-pro-preview, gemini-3.1-pro-preview, gpt-4.1, gpt-5.2-high-reasoning, gpt-5.3-codex, gpt-5.4, haiku-4.5, kimi-2.5, kimi-2.5-openrouter, opus-4.5, opus-4.6, qwen-next-80B-instruct, qwen-next-80B-thinking, sonnet-4.5, sonnet-4.6
Commands: /eval · /rerun · /list
CLI: gh workflow run eval-regression.yaml --repo HolmesGPT/holmesgpt --ref claude/add-dr-test-log-check-oAq7y -f markers=regression -f filter=
WalkthroughAdded two new test fixture files to set up an Elasticsearch-based test scenario for validating Holmes' behavior with DR log checks. The test case defines a user prompt querying Elasticsearch for specific service errors, while the toolsets configuration restricts execution to Elasticsearch integrations only. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~5 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
|
✅ Docker images ready for
Use these tags to pull the images for testing. 📋 Copy commandsgcloud auth configure-docker us-central1-docker.pkg.dev
docker pull us-central1-docker.pkg.dev/robusta-development/temporary-builds/holmes:97892dc2
docker tag us-central1-docker.pkg.dev/robusta-development/temporary-builds/holmes:97892dc2 me-west1-docker.pkg.dev/robusta-development/development/holmes-dev:97892dc2
docker push me-west1-docker.pkg.dev/robusta-development/development/holmes-dev:97892dc2
docker pull us-central1-docker.pkg.dev/robusta-development/temporary-builds/holmes-operator:97892dc2
docker tag us-central1-docker.pkg.dev/robusta-development/temporary-builds/holmes-operator:97892dc2 me-west1-docker.pkg.dev/robusta-development/development/holmes-operator-dev:97892dc2
docker push me-west1-docker.pkg.dev/robusta-development/development/holmes-operator-dev:97892dc2Patch 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:97892dc2 \
--set operator.registry=me-west1-docker.pkg.dev/robusta-development/development \
--set operator.image=holmes-operator-dev:97892dc2Robusta 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:97892dc2 \
--set holmes.operator.registry=me-west1-docker.pkg.dev/robusta-development/development \
--set holmes.operator.image=holmes-operator-dev:97892dc2 |
✅ Deploy Preview for holmes-docs ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tests/llm/fixtures/test_ask_holmes/254_elasticsearch_dr_test_log_check/test_case.yaml (1)
27-27: Consider whetherinclude_tool_calls: trueis necessary.The expected output assertions are fairly specific (must state no data found, must NOT claim errors from other services explain the issue). Consider whether the assertions alone are sufficient to rule out hallucinations, or whether tool call visibility is needed to verify Holmes actually queried Elasticsearch and processed the results correctly.
The negative assertions ("Must NOT claim...") might benefit from tool call inspection to confirm Holmes found the other services but correctly distinguished them. As per coding guidelines, prefer specific value checking when possible, but use
include_tool_calls: truewhen needed to prevent hallucinations.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/llm/fixtures/test_ask_holmes/254_elasticsearch_dr_test_log_check/test_case.yaml` at line 27, The test currently sets include_tool_calls: true which may be unnecessary; either remove this key from test_case.yaml if the existing expected-output assertions (the positive "no data found" assertion and the negative "Must NOT claim..." checks) are sufficient, or keep include_tool_calls: true but add explicit tool-call assertions verifying the Holmes agent invoked the Elasticsearch tool and that its returned result was processed (e.g., assert tool_calls contains an elasticsearch query and a subsequent processed result) so the test proves Holmes actually queried ES rather than hallucinating; update the file by removing include_tool_calls when not needed, or by keeping it and adding precise tool-call checks to confirm Elasticsearch was queried and distinguished from other services.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@tests/llm/fixtures/test_ask_holmes/254_elasticsearch_dr_test_log_check/test_case.yaml`:
- Line 27: The test currently sets include_tool_calls: true which may be
unnecessary; either remove this key from test_case.yaml if the existing
expected-output assertions (the positive "no data found" assertion and the
negative "Must NOT claim..." checks) are sufficient, or keep include_tool_calls:
true but add explicit tool-call assertions verifying the Holmes agent invoked
the Elasticsearch tool and that its returned result was processed (e.g., assert
tool_calls contains an elasticsearch query and a subsequent processed result) so
the test proves Holmes actually queried ES rather than hallucinating; update the
file by removing include_tool_calls when not needed, or by keeping it and adding
precise tool-call checks to confirm Elasticsearch was queried and distinguished
from other services.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 826aad70-c750-438c-946d-3847fd537140
📒 Files selected for processing (2)
tests/llm/fixtures/test_ask_holmes/254_elasticsearch_dr_test_log_check/test_case.yamltests/llm/fixtures/test_ask_holmes/254_elasticsearch_dr_test_log_check/toolsets.yaml
## Summary This PR adds a new test case that validates Holmes correctly identifies when it lacks log data for a specific service and avoids making incorrect diagnoses based on unrelated services' logs. ## Key Changes - **New test case**: `254_elasticsearch_dr_test_log_check` - Tests Holmes' ability to recognize missing service data - Scenario: Customer reports `companyopswebjob` failures during a DR test - Test data includes logs for `Company.Ops` and `Company.Ops.Radar.WebJob` (unrelated services) with job processing failures - Deliberately excludes any logs for `companyopswebjob` to test data transparency - **Test expectations**: Holmes must: - Explicitly state that no log data exists for `companyopswebjob` - NOT attribute errors from unrelated services to the missing service - NOT provide root cause diagnosis based on unrelated service logs - Recommend obtaining logs specifically from `companyopswebjob` - **Test configuration**: - Uses Elasticsearch-only toolset (Kubernetes, Helm, Robusta, and Bash disabled) - Creates test index with 12 log records across 2 services - Verifies setup with document count and service presence checks - Includes cross-platform timestamp generation for relative time queries ## Implementation Details - Test data simulates a realistic DR failover scenario with database connection issues - Deliberately includes "red herring" errors in similar-named services to test Holmes' discrimination - Uses bulk indexing with proper index refresh and verification - Implements idempotent cleanup strategy for parallel CI execution safety https://claude.ai/code/session_018QqVoFkVGpSkSAK4SzgzEN <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Tests** * Added new test fixtures for Elasticsearch Disaster Recovery log checking scenarios * Tests verify Holmes accurately identifies missing log data and avoids misattributing errors from unrelated services * Validates Elasticsearch integration with proper toolset configuration <!-- end of auto-generated comment: release notes by coderabbit.ai --> Signed-off-by: Claude <noreply@anthropic.com> Co-authored-by: Claude <noreply@anthropic.com>
Summary
This PR adds a new test case that validates Holmes correctly identifies when it lacks log data for a specific service and avoids making incorrect diagnoses based on unrelated services' logs.
Key Changes
New test case:
254_elasticsearch_dr_test_log_check- Tests Holmes' ability to recognize missing service datacompanyopswebjobfailures during a DR testCompany.OpsandCompany.Ops.Radar.WebJob(unrelated services) with job processing failurescompanyopswebjobto test data transparencyTest expectations: Holmes must:
companyopswebjobcompanyopswebjobTest configuration:
Implementation Details
https://claude.ai/code/session_018QqVoFkVGpSkSAK4SzgzEN
Summary by CodeRabbit