Conversation
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📥 CommitsReviewing files that changed from the base of the PR and between 920936a8f1147479035ec3fade26c2eb64520060 and 9a4af76. 📒 Files selected for processing (7)
✅ Files skipped from review due to trivial changes (6)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughAdded an Changes
Sequence Diagram(s)sequenceDiagram
participant Tester as Tester (test harness)
participant ES as Elasticsearch
participant Holmes as Holmes LLM
Tester->>ES: delete index (if exists) / create index with mappings
Tester->>ES: bulk index test documents
Tester->>ES: refresh index and run count/validation queries
Tester->>Holmes: invoke test prompt (query logs via ES toolset)
Holmes->>ES: query logs (via elasticsearch/data or cluster toolset)
Holmes-->>Tester: return analysis (must hedge / provide hypotheses)
Tester->>ES: delete test index (teardown)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 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 |
❌ Deploy Preview for holmes-docs failed. Why did it fail? →
|
✅ Results of HolmesGPT evalsAutomatically triggered by commit 9a4af76 on branch Results of HolmesGPT evals
Benchmark comparison unavailable: No ci-benchmark experiments found Benchmark Comparison DetailsBaseline: latest ci-benchmark experiment on master Status: No ci-benchmark experiments found Comparison indicators:
📖 Legend
🔄 Re-run evals manually
Option 1: Comment on this PR with Or with more options (one per line): Run evals on a different branch (e.g., master) for comparison:
Quick re-run: Use Option 2: Trigger via GitHub Actions UI → "Run workflow" Option 3: Add PR labels to include extra evals (applies to both automatic runs and
Examples: 🏷️ Valid tags
🤖 Valid models
Commands: CLI: |
|
✅ 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:79e569ff
docker tag us-central1-docker.pkg.dev/robusta-development/temporary-builds/holmes:79e569ff me-west1-docker.pkg.dev/robusta-development/development/holmes-dev:79e569ff
docker push me-west1-docker.pkg.dev/robusta-development/development/holmes-dev:79e569ff
docker pull us-central1-docker.pkg.dev/robusta-development/temporary-builds/holmes-operator:79e569ff
docker tag us-central1-docker.pkg.dev/robusta-development/temporary-builds/holmes-operator:79e569ff me-west1-docker.pkg.dev/robusta-development/development/holmes-operator-dev:79e569ff
docker push me-west1-docker.pkg.dev/robusta-development/development/holmes-operator-dev:79e569ffPatch 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:79e569ff \
--set operator.registry=me-west1-docker.pkg.dev/robusta-development/development \
--set operator.image=holmes-operator-dev:79e569ffRobusta 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:79e569ff \
--set holmes.operator.registry=me-west1-docker.pkg.dev/robusta-development/development \
--set holmes.operator.image=holmes-operator-dev:79e569ff |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tests/llm/fixtures/test_ask_holmes/212_elasticsearch_overconfidence_incomplete_evidence/test_case.yaml (1)
127-141: Consider adding total document count verification for consistency.Tests 213 and 214 verify both total document count and specific filtered counts, but this test only verifies the error count. While verifying
ERROR_COUNT=6is sufficient for test data integrity, adding a total count check (expecting 15 documents) would improve consistency across the test suite and catch potential bulk insert issues.Optional: Add total document count verification
echo "Test index created with $DOC_COUNT total logs ($ERROR_COUNT with 500 status)" + if [ "$DOC_COUNT" != "15" ]; then + echo "Expected 15 total logs but found: $DOC_COUNT" + exit 1 + fi + if [ "$ERROR_COUNT" != "6" ]; then echo "Expected 6 error logs but found: $ERROR_COUNT" exit 1 fi🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/llm/fixtures/test_ask_holmes/212_elasticsearch_overconfidence_incomplete_evidence/test_case.yaml` around lines 127 - 141, Add a verification that the total document count (DOC_COUNT) equals 15 before or alongside the existing ERROR_COUNT check: after computing DOC_COUNT (from the curl to "${ELASTICSEARCH_URL}/${HOLMES_ES_TEST_INDEX}/_count"), compare it to "15" and if it does not match, echo a descriptive message and exit 1; keep existing ERROR_COUNT logic intact so both DOC_COUNT and ERROR_COUNT (6) are validated for the test.
🤖 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/212_elasticsearch_overconfidence_incomplete_evidence/test_case.yaml`:
- Around line 127-141: Add a verification that the total document count
(DOC_COUNT) equals 15 before or alongside the existing ERROR_COUNT check: after
computing DOC_COUNT (from the curl to
"${ELASTICSEARCH_URL}/${HOLMES_ES_TEST_INDEX}/_count"), compare it to "15" and
if it does not match, echo a descriptive message and exit 1; keep existing
ERROR_COUNT logic intact so both DOC_COUNT and ERROR_COUNT (6) are validated for
the test.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 8f9dcca8-8cee-4441-ad06-5a772ee69ae6
📒 Files selected for processing (7)
pyproject.tomltests/llm/fixtures/test_ask_holmes/212_elasticsearch_overconfidence_incomplete_evidence/test_case.yamltests/llm/fixtures/test_ask_holmes/212_elasticsearch_overconfidence_incomplete_evidence/toolsets.yamltests/llm/fixtures/test_ask_holmes/213_elasticsearch_overconfidence_ambiguous_cause/test_case.yamltests/llm/fixtures/test_ask_holmes/213_elasticsearch_overconfidence_ambiguous_cause/toolsets.yamltests/llm/fixtures/test_ask_holmes/214_elasticsearch_overconfidence_correlation_not_causation/test_case.yamltests/llm/fixtures/test_ask_holmes/214_elasticsearch_overconfidence_correlation_not_causation/toolsets.yaml
🔬 CLI Performance Benchmark🟢 Startup Time (no LLM)Measures
🟡 Full CLI with LLMMeasures
PR: |
Three new Elasticsearch-based evals testing whether the LLM expresses appropriate uncertainty when evidence is insufficient or ambiguous: - 212: Incomplete evidence - HTTP access logs show 500 errors but contain no application-level diagnostics. LLM should not fabricate root causes. Fails 3/3 on Opus 4.5 (model confidently claims "upstream timeout" as root cause despite having only status codes and latencies). - 213: Ambiguous cause - Three services fail simultaneously with no dependency topology info. LLM should present multiple hypotheses. Fails 3/3 on Opus 4.5 (model picks one service as THE definitive root cause based on 1-second timestamp differences). - 214: Correlation not causation - Deployment of one service coincides temporally with errors in an unrelated service. LLM should note correlation without claiming causation. Passes 3/3 on Opus 4.5 (included as calibration baseline). Also adds 'overconfidence' pytest marker to pyproject.toml. https://claude.ai/code/session_01JqXGb7TpzRuZgnKDrzgMXf Signed-off-by: Claude <noreply@anthropic.com>
920936a to
9a4af76
Compare
This branch has not been deployed
2 participants
Summary
This PR adds three new test cases to the Holmes LLM test suite that validate the system's ability to express appropriate uncertainty and avoid overconfident conclusions when analyzing incomplete or ambiguous data.
Key Changes
New Test Cases
Test 212 - Incomplete Evidence: Validates that Holmes acknowledges HTTP 500 errors but expresses uncertainty about root cause when only HTTP access logs are available (no application logs, traces, or diagnostics)
Test 213 - Ambiguous Root Cause: Tests that Holmes presents multiple hypotheses rather than claiming a single definitive root cause when three services fail simultaneously with no dependency topology information
Test 214 - Correlation Not Causation: Ensures Holmes doesn't assume a deployment caused errors in another service just because they occurred in temporal proximity, without evidence of a causal link
Implementation Details
Each test includes:
test_case.yamlfile with setup/teardown logic that creates Elasticsearch indices with realistic log datatoolsets.yamlconfiguration enabling Elasticsearch data and cluster tools while disabling Kubernetes-specific toolsTest data is carefully crafted to:
Expected outputs use specific language requirements to validate appropriate uncertainty:
These tests help ensure Holmes maintains calibrated confidence levels and avoids the common LLM pitfall of fabricating explanations when data is incomplete or ambiguous.
https://claude.ai/code/session_01JqXGb7TpzRuZgnKDrzgMXf
Summary by CodeRabbit