Skip to content

Improve 'close-matches' naming behaviour - #2042

Closed
aantn wants to merge 16 commits into
masterfrom
claude/test-eval-254-models-MAagl
Closed

aantn wants to merge 16 commits into
masterfrom
claude/test-eval-254-models-MAagl

Conversation

@aantn

@aantn aantn commented May 14, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

Updated the Elasticsearch disaster recovery test case to include explicit instructions for service field validation, ensuring the LLM only considers logs from the companyopswebjob service.

Changes

  • Added clarifying instructions to the user prompt in the test case that:
    • Require exact matching of the service field to companyopswebjob (case-insensitive)
    • Prohibit substitution or inference from logs of other services with similar names
    • Mandate explicit acknowledgment if no logs for companyopswebjob are found
    • Prevent proposing root causes from unrelated services
    • Recommend obtaining logs directly from companyopswebjob if unavailable

Details

This change improves the test case by making the service filtering requirement explicit in the prompt, which helps validate that the LLM correctly handles cases where it must distinguish between services and avoid making assumptions based on partial name matches or logs from different services.

https://claude.ai/code/session_015scgyQbqJD23heqct31Ger

Summary by CodeRabbit

  • New Features

    • Assistant now explicitly states when no data exists for the exact requested entity, may surface findings from similarly named/related entities only when clearly labeled as different, and prompts users to confirm or provide the intended entity. It also flags cluster/source mismatches when relevant.
  • Tests

    • Updated and added test fixtures to enforce these transparency rules, including a new wrong-cluster scenario and added a regression tag to relevant tests.

Review Change Stack

Original prompt let the LLM attribute errors from sibling services
(Company.Ops, Company.Ops.Radar.WebJob) to companyopswebjob. Adding an
explicit instruction to only consider exact service matches and to
state when no logs exist makes the eval pass on Sonnet 4.5, Opus 4.6,
and Opus 4.7.

Signed-off-by: Claude <noreply@anthropic.com>

@claude claude 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.

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.

@github-actions

github-actions Bot commented May 14, 2026 •

Copy link
Copy Markdown
Contributor

📂 Previous Runs

⚠️ 1 older run truncated

Older runs were omitted to stay under GitHub's 64KB comment size limit.


✅ Results of HolmesGPT evals

Automatically triggered by commit 5ecfea0 on branch claude/test-eval-254-models-MAagl (labels: evals-tag-regression, evals-tag-multi-cluster)

View workflow logs

Results of HolmesGPT evals

  • ask_holmes: 24/24 test cases were successful, 0 regressions
Status Test case Time Turns Tools Cost Total tokens Input Max input Output Max output Cached Non-cached Reasoning Compactions Src
✅ 09_crashpod 36.5s 5 11 $0.2783 108,838 106,608 24,103 2,230 882 81,306 25,302 188 — src
✅ 101_loki_historical_logs_pod_deleted 86.7s 7 17 $0.4331 181,837 176,910 30,579 4,927 1,065 144,936 31,974 1,146 — src
✅ 112_find_pvcs_by_uuid 20.3s 3 4 $0.2093 62,157 60,919 22,216 1,238 626 38,447 22,472 297 — src
✅ 12_job_crashing 39.3s 5 13 $0.2979 111,740 109,279 25,017 2,461 1,049 81,862 27,417 154 — src
✅ 176_network_policy_blocking_traffic_no_skills 44.7s 5 13 $0.3243 112,612 109,707 26,224 2,905 952 79,799 29,908 402 — src
✅ 227_count_configmaps_per_namespace[0] 20.1s 4 9 $0.2114 78,021 76,867 20,986 1,154 593 54,944 21,923 74 — src
✅ 243_pod_names_contain_service 35.7s 4 10 $0.2575 83,814 81,551 23,173 2,263 958 57,602 23,949 249 — src
✅ 24_misconfigured_pvc 42.2s 5 14 $0.3022 110,769 108,037 24,748 2,732 1,076 81,047 26,990 352 — src
✅ 254_elasticsearch_dr_test_log_check 77.2s 10 16 $0.3902 175,876 170,575 23,120 5,301 951 146,563 24,012 885 — src
✅ 259_wrong_cluster_logs_confusion 70.6s 7 9 $0.3151 126,217 122,126 21,200 4,091 906 100,651 21,475 1,047 — src
✅ 260_global_es_remote_cluster_logs 99.1s 10 19 $0.4887 215,596 209,113 28,124 6,483 1,523 177,447 31,666 915 — src
✅ 261_time_window_gap_external_data 104.3s 12 19 $0.5022 269,222 263,372 30,411 5,850 875 232,123 31,249 1,131 — src
✅ 262_ambiguous_cluster_reference 63.4s 7 13 $0.3250 130,903 126,727 21,740 4,176 959 104,387 22,340 832 — src
✅ 263_region_suffixed_service_siblings 73.6s 11 14 $0.3580 192,869 188,685 21,402 4,184 804 166,518 22,167 591 — src
✅ 264_wrong_env_same_region 55.7s 7 9 $0.2665 117,490 114,532 18,873 2,958 630 95,095 19,437 630 — src
✅ 265_multicluster_labeling_discipline 82.0s 8 15 $0.3842 161,808 156,685 23,915 5,123 1,303 131,585 25,100 1,013 — src
✅ 266_toolset_disabled_vs_no_data 9.6s 1 — $0.1025 13,710 13,310 13,310 400 400 0 13,310 125 — src
✅ 267_cluster_name_alias 82.5s 11 21 $0.4423 221,937 216,515 26,344 5,422 959 188,665 27,850 486 — src
✅ 268_kubectl_wrong_cluster 55.2s 5 9 $0.3123 115,780 112,658 25,340 3,122 913 86,467 26,191 795 — src
✅ 269_mixed_sources_route_to_external 76.3s 9 15 $0.3809 187,326 182,880 24,740 4,446 1,067 157,825 25,055 827 — src
✅ 270_namespace_collision 34.0s 4 10 $0.2690 91,484 89,401 25,540 2,083 872 63,650 25,751 297 — src
✅ 43_current_datetime_from_prompt 4.0s 1 — $0.0124 17,312 17,207 17,207 105 105 17,204 3 64 — src
✅ 51_logs_summarize_errors 23.5s 4 5 $0.2083 78,278 77,109 21,138 1,169 380 55,966 21,143 91 — src
✅ 61_exact_match_counting 11.0s 3 3 $0.1543 53,654 53,291 18,184 363 216 35,103 18,188 32 — src
Total 52.0s avg 6.2 avg 12.2 avg $7.2260 3,019,250 2,944,064 30,579 75,186 1,523 2,379,192 564,872 12,623 —
Benchmark Comparison Details

Master baseline: latest master-* experiment (post-merge regression eval)
Status: 11 test/model combinations loaded

Benchmark baseline: latest ci-benchmark experiment on master
Status: 177 test/model combinations loaded

Time comparison (seconds):

Test case This branch master (1d ago) Δ vs master benchmark (4d ago) Δ vs benchmark
09_crashpod (opus-4.6) 📄 36.5s 39.5s ±0% 45.0s ↓19%
101_loki_historical_logs_pod_deleted (opus-4.6) 📄 86.7s 84.3s ±0% 85.9s ±0%
112_find_pvcs_by_uuid (opus-4.6) 📄 20.3s 22.4s ±0% 21.4s ±0%
12_job_crashing (opus-4.6) 📄 39.3s 49.0s ↓20% 38.6s ±0%
176_network_policy_blocking_traffic_no_skills (opus-4.6) 📄 44.7s 66.5s ↓33% 53.9s ↓17%
227_count_configmaps_per_namespace[0] (opus-4.6) 📄 20.1s 21.7s ±0% 20.8s ±0%
243_pod_names_contain_service (opus-4.6) 📄 35.7s 35.0s ±0% 33.6s ±0%
24_misconfigured_pvc (opus-4.6) 📄 42.2s 45.3s ±0% 37.0s ↑14%
254_elasticsearch_dr_test_log_check (opus-4.6) 📄 77.2s — — — —
259_wrong_cluster_logs_confusion (opus-4.6) 📄 70.6s — — — —
260_global_es_remote_cluster_logs (opus-4.6) 📄 99.1s — — — —
261_time_window_gap_external_data (opus-4.6) 📄 104.3s — — — —
262_ambiguous_cluster_reference (opus-4.6) 📄 63.4s — — — —
263_region_suffixed_service_siblings (opus-4.6) 📄 73.6s — — — —
264_wrong_env_same_region (opus-4.6) 📄 55.7s — — — —
265_multicluster_labeling_discipline (opus-4.6) 📄 82.0s — — — —
266_toolset_disabled_vs_no_data (opus-4.6) 📄 9.6s — — — —
267_cluster_name_alias (opus-4.6) 📄 82.5s — — — —
268_kubectl_wrong_cluster (opus-4.6) 📄 55.2s — — — —
269_mixed_sources_route_to_external (opus-4.6) 📄 76.3s — — — —
270_namespace_collision (opus-4.6) 📄 34.0s — — — —
43_current_datetime_from_prompt (opus-4.6) 📄 4.0s 3.5s ↑14% 3.6s ↑11%
51_logs_summarize_errors (opus-4.6) 📄 23.5s 22.9s ±0% 21.9s ±0%
61_exact_match_counting (opus-4.6) 📄 11.0s 11.6s ±0% 10.0s ↑10%
Total (all, n=24) 52.0s 36.5s — 33.8s —
Comparable (m=11, b=11) 33.1s 36.5s ±0% 33.8s ±0%

Cost comparison:

Test case This branch master (1d ago) Δ vs master benchmark (4d ago) Δ vs benchmark
09_crashpod (opus-4.6) 📄 $0.2783 $0.2949 ±0% $0.3318 ↓16%
101_loki_historical_logs_pod_deleted (opus-4.6) 📄 $0.4331 $0.4098 ±0% $0.4631 ±0%
112_find_pvcs_by_uuid (opus-4.6) 📄 $0.2093 $0.1861 ↑12% $0.2066 ±0%
12_job_crashing (opus-4.6) 📄 $0.2979 $0.3288 ±0% $0.3107 ±0%
176_network_policy_blocking_traffic_no_skills (opus-4.6) 📄 $0.3243 $0.3783 ↓14% $0.3426 ±0%
227_count_configmaps_per_namespace[0] (opus-4.6) 📄 $0.2114 $0.2031 ±0% $0.2031 ±0%
243_pod_names_contain_service (opus-4.6) 📄 $0.2575 $0.2504 ±0% $0.2521 ±0%
24_misconfigured_pvc (opus-4.6) 📄 $0.3022 $0.3180 ±0% $0.2904 ±0%
254_elasticsearch_dr_test_log_check (opus-4.6) 📄 $0.3902 — — — —
259_wrong_cluster_logs_confusion (opus-4.6) 📄 $0.3151 — — — —
260_global_es_remote_cluster_logs (opus-4.6) 📄 $0.4887 — — — —
261_time_window_gap_external_data (opus-4.6) 📄 $0.5022 — — — —
262_ambiguous_cluster_reference (opus-4.6) 📄 $0.3250 — — — —
263_region_suffixed_service_siblings (opus-4.6) 📄 $0.3580 — — — —
264_wrong_env_same_region (opus-4.6) 📄 $0.2665 — — — —
265_multicluster_labeling_discipline (opus-4.6) 📄 $0.3842 — — — —
266_toolset_disabled_vs_no_data (opus-4.6) 📄 $0.1025 — — — —
267_cluster_name_alias (opus-4.6) 📄 $0.4423 — — — —
268_kubectl_wrong_cluster (opus-4.6) 📄 $0.3123 — — — —
269_mixed_sources_route_to_external (opus-4.6) 📄 $0.3809 — — — —
270_namespace_collision (opus-4.6) 📄 $0.2690 — — — —
43_current_datetime_from_prompt (opus-4.6) 📄 $0.0124 $0.1190 ↓90% $0.1187 ↓90%
51_logs_summarize_errors (opus-4.6) 📄 $0.2083 $0.2049 ±0% $0.2015 ±0%
61_exact_match_counting (opus-4.6) 📄 $0.1543 $0.1518 ±0% $0.1518 ±0%
Total (all, n=24) $0.3011 $0.2587 — $0.2611 —
Comparable (m=11, b=11) $0.2445 $0.2587 ±0% $0.2611 ±0%

Total tokens comparison:

Test case This branch master (1d ago) Δ vs master benchmark (4d ago) Δ vs benchmark
09_crashpod (opus-4.6) 📄 108,838 131,686 ↓17% 156,516 ↓30%
101_loki_historical_logs_pod_deleted (opus-4.6) 📄 181,837 171,379 ±0% 204,760 ↓11%
112_find_pvcs_by_uuid (opus-4.6) 📄 62,157 56,309 ↑10% 61,198 ±0%
12_job_crashing (opus-4.6) 📄 111,740 159,927 ↓30% 135,931 ↓18%
176_network_policy_blocking_traffic_no_skills (opus-4.6) 📄 112,612 150,279 ↓25% 140,458 ↓20%
227_count_configmaps_per_namespace[0] (opus-4.6) 📄 78,021 76,668 ±0% 76,661 ±0%
243_pod_names_contain_service (opus-4.6) 📄 83,814 81,854 ±0% 82,231 ±0%
24_misconfigured_pvc (opus-4.6) 📄 110,769 134,154 ↓17% 107,468 ±0%
254_elasticsearch_dr_test_log_check (opus-4.6) 📄 175,876 — — — —
259_wrong_cluster_logs_confusion (opus-4.6) 📄 126,217 — — — —
260_global_es_remote_cluster_logs (opus-4.6) 📄 215,596 — — — —
261_time_window_gap_external_data (opus-4.6) 📄 269,222 — — — —
262_ambiguous_cluster_reference (opus-4.6) 📄 130,903 — — — —
263_region_suffixed_service_siblings (opus-4.6) 📄 192,869 — — — —
264_wrong_env_same_region (opus-4.6) 📄 117,490 — — — —
265_multicluster_labeling_discipline (opus-4.6) 📄 161,808 — — — —
266_toolset_disabled_vs_no_data (opus-4.6) 📄 13,710 — — — —
267_cluster_name_alias (opus-4.6) 📄 221,937 — — — —
268_kubectl_wrong_cluster (opus-4.6) 📄 115,780 — — — —
269_mixed_sources_route_to_external (opus-4.6) 📄 187,326 — — — —
270_namespace_collision (opus-4.6) 📄 91,484 — — — —
43_current_datetime_from_prompt (opus-4.6) 📄 17,312 17,001 ±0% 16,989 ±0%
51_logs_summarize_errors (opus-4.6) 📄 78,278 77,316 ±0% 76,727 ±0%
61_exact_match_counting (opus-4.6) 📄 53,654 52,721 ±0% 52,723 ±0%
Total (all, n=24) 125,802 100,845 — 101,060 —
Comparable (m=11, b=11) 90,821 100,845 ±0% 101,060 ↓10%

Cached tokens comparison:

Test case This branch master (1d ago) Δ vs master benchmark (4d ago) Δ vs benchmark
09_crashpod (opus-4.6) 📄 81,306 104,283 ↓22% 126,090 ↓36%
101_loki_historical_logs_pod_deleted (opus-4.6) 📄 144,936 137,233 ±0% 166,234 ↓13%
112_find_pvcs_by_uuid (opus-4.6) 📄 38,447 35,446 ±0% 37,847 ±0%
12_job_crashing (opus-4.6) 📄 81,862 130,712 ↓37% 106,794 ↓23%
176_network_policy_blocking_traffic_no_skills (opus-4.6) 📄 79,799 115,477 ↓31% 108,926 ↓27%
227_count_configmaps_per_namespace[0] (opus-4.6) 📄 54,944 54,891 ±0% 54,888 ±0%
243_pod_names_contain_service (opus-4.6) 📄 57,602 56,021 ±0% 56,547 ±0%
24_misconfigured_pvc (opus-4.6) 📄 81,047 104,830 ↓23% 78,838 ±0%
254_elasticsearch_dr_test_log_check (opus-4.6) 📄 146,563 — — — —
259_wrong_cluster_logs_confusion (opus-4.6) 📄 100,651 — — — —
260_global_es_remote_cluster_logs (opus-4.6) 📄 177,447 — — — —
261_time_window_gap_external_data (opus-4.6) 📄 232,123 — — — —
262_ambiguous_cluster_reference (opus-4.6) 📄 104,387 — — — —
263_region_suffixed_service_siblings (opus-4.6) 📄 166,518 — — — —
264_wrong_env_same_region (opus-4.6) 📄 95,095 — — — —
265_multicluster_labeling_discipline (opus-4.6) 📄 131,585 — — — —
266_toolset_disabled_vs_no_data (opus-4.6) 📄 — — — — —
267_cluster_name_alias (opus-4.6) 📄 188,665 — — — —
268_kubectl_wrong_cluster (opus-4.6) 📄 86,467 — — — —
269_mixed_sources_route_to_external (opus-4.6) 📄 157,825 — — — —
270_namespace_collision (opus-4.6) 📄 63,650 — — — —
43_current_datetime_from_prompt (opus-4.6) 📄 17,204 — — — —
51_logs_summarize_errors (opus-4.6) 📄 55,966 55,204 ±0% 54,936 ±0%
61_exact_match_counting (opus-4.6) 📄 35,103 34,482 ±0% 34,484 ±0%
Total (all, n=24) 99,133 75,325 — 75,053 —
Comparable (m=10, b=10) 71,101 82,858 ↓14% 82,558 ↓14%

Turns comparison:

Test case This branch master (1d ago) Δ vs master benchmark (4d ago) Δ vs benchmark
09_crashpod (opus-4.6) 📄 5 6 ↓17% 7 ↓29%
101_loki_historical_logs_pod_deleted (opus-4.6) 📄 7 7 ±0% 8 ↓12%
112_find_pvcs_by_uuid (opus-4.6) 📄 3 3 ±0% 3 ±0%
12_job_crashing (opus-4.6) 📄 5 7 ↓29% 6 ↓17%
176_network_policy_blocking_traffic_no_skills (opus-4.6) 📄 5 6 ↓17% 6 ↓17%
227_count_configmaps_per_namespace[0] (opus-4.6) 📄 4 4 ±0% 4 ±0%
243_pod_names_contain_service (opus-4.6) 📄 4 4 ±0% 4 ±0%
24_misconfigured_pvc (opus-4.6) 📄 5 6 ↓17% 5 ±0%
254_elasticsearch_dr_test_log_check (opus-4.6) 📄 10 — — — —
259_wrong_cluster_logs_confusion (opus-4.6) 📄 7 — — — —
260_global_es_remote_cluster_logs (opus-4.6) 📄 10 — — — —
261_time_window_gap_external_data (opus-4.6) 📄 12 — — — —
262_ambiguous_cluster_reference (opus-4.6) 📄 7 — — — —
263_region_suffixed_service_siblings (opus-4.6) 📄 11 — — — —
264_wrong_env_same_region (opus-4.6) 📄 7 — — — —
265_multicluster_labeling_discipline (opus-4.6) 📄 8 — — — —
266_toolset_disabled_vs_no_data (opus-4.6) 📄 1 — — — —
267_cluster_name_alias (opus-4.6) 📄 11 — — — —
268_kubectl_wrong_cluster (opus-4.6) 📄 5 — — — —
269_mixed_sources_route_to_external (opus-4.6) 📄 9 — — — —
270_namespace_collision (opus-4.6) 📄 4 — — — —
43_current_datetime_from_prompt (opus-4.6) 📄 1 1 ±0% 1 ±0%
51_logs_summarize_errors (opus-4.6) 📄 4 4 ±0% 4 ±0%
61_exact_match_counting (opus-4.6) 📄 3 3 ±0% 3 ±0%
Total (all, n=24) 6.2 4.6 — 4.6 —
Comparable (m=11, b=11) 4.2 4.6 ±0% 4.6 ±0%

Tool calls comparison:

Test case This branch master (1d ago) Δ vs master benchmark (4d ago) Δ vs benchmark
09_crashpod (opus-4.6) 📄 11 12 ±0% 13 ↓15%
101_loki_historical_logs_pod_deleted (opus-4.6) 📄 17 18 ±0% 18 ±0%
112_find_pvcs_by_uuid (opus-4.6) 📄 4 4 ±0% 4 ±0%
12_job_crashing (opus-4.6) 📄 13 14 ±0% 14 ±0%
176_network_policy_blocking_traffic_no_skills (opus-4.6) 📄 13 18 ↓28% 14 ±0%
227_count_configmaps_per_namespace[0] (opus-4.6) 📄 9 9 ±0% 9 ±0%
243_pod_names_contain_service (opus-4.6) 📄 10 10 ±0% 10 ±0%
24_misconfigured_pvc (opus-4.6) 📄 14 16 ↓12% 15 ±0%
254_elasticsearch_dr_test_log_check (opus-4.6) 📄 16 — — — —
259_wrong_cluster_logs_confusion (opus-4.6) 📄 9 — — — —
260_global_es_remote_cluster_logs (opus-4.6) 📄 19 — — — —
261_time_window_gap_external_data (opus-4.6) 📄 19 — — — —
262_ambiguous_cluster_reference (opus-4.6) 📄 13 — — — —
263_region_suffixed_service_siblings (opus-4.6) 📄 14 — — — —
264_wrong_env_same_region (opus-4.6) 📄 9 — — — —
265_multicluster_labeling_discipline (opus-4.6) 📄 15 — — — —
266_toolset_disabled_vs_no_data (opus-4.6) 📄 — — — — —
267_cluster_name_alias (opus-4.6) 📄 21 — — — —
268_kubectl_wrong_cluster (opus-4.6) 📄 9 — — — —
269_mixed_sources_route_to_external (opus-4.6) 📄 15 — — — —
270_namespace_collision (opus-4.6) 📄 10 — — — —
43_current_datetime_from_prompt (opus-4.6) 📄 — — — — —
51_logs_summarize_errors (opus-4.6) 📄 5 5 ±0% 5 ±0%
61_exact_match_counting (opus-4.6) 📄 3 3 ±0% 3 ±0%
Total (all, n=24) 11.2 10.9 — 10.5 —
Comparable (m=10, b=10) 9.9 10.9 ±0% 10.5 ±0%

Comparison indicators:

  • ±0% — diff under 10% (within noise threshold)
  • ↑N%/↓N% — diff 10-25%
  • ↑N%/↓N% — diff over 25% (significant)
📖 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: /eval comments 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/test-eval-254-models-MAagl -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, conversation_worker, coralogix, counting, database, datadog, datetime, db-connectors, easy, elasticsearch, embeds, fast, frontend, grafana, hard, images, integration, kafka, kubernetes, leaked-information, logs, loki, manual, mcp, medium, metrics, multi-cluster, network, newrelic, no-cicd, numerical, one-test, port-forward, prometheus, question-answer, regression, skills, slackbot, storage, token-limit, toolset-limitation, traces, transparency, victorialogs

🤖 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, opus-4.7, 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/test-eval-254-models-MAagl -f markers=regression -f filter=

@coderabbitai

coderabbitai Bot commented May 14, 2026 •

Copy link
Copy Markdown
Contributor

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

Walkthrough

Adds a prompt instruction for transparent adjacent/similarly-named entity reporting and updates/introduces eval fixtures to require explicit "no data for exact name" messaging, labeled use of related entities, and explicit cluster-mismatch disclosure.

Changes

Prompt guidance

Layer / File(s) Summary
Adjacent / similarly-named entities instruction
holmes/plugins/prompts/generic_ask.jinja2
New instruction block requiring explicit statement that the exact provided entity had no data, guidance to report findings from closest related entities only when clearly labeled as different, and a prompt to ask the user to verify or provide the correct entity.

Test fixtures enforcing transparent adjacent reporting

Layer / File(s) Summary
DR Elasticsearch log-check fixture: explicit no-data + labeled adjacent findings
tests/llm/fixtures/test_ask_holmes/254_elasticsearch_dr_test_log_check/test_case.yaml
Rewrote scenario and expected_output to require (1) explicit "no logs found" for companyopswebjob, (2) reporting job-failure findings from Company.Ops and/or Company.Ops.Radar.WebJob within the DR window only when clearly labeled as different, and (3) disclosure that any analysis/root-cause is based on those related services; added regression tag.
Missing-app-details fixture: explicit not-found + labeled closest-match
tests/llm/fixtures/test_ask_holmes/19_detect_missing_app_details/test_case.yaml
Updated expected_output to require both (1) explicit statement that personal-certs-validator was not found and (2) surfacing db-certs-authenticator information while clearly labeling it as a different resource.
Wrong-cluster confusion fixture (prompt, tags, expectations, setup)
tests/llm/fixtures/test_ask_holmes/259_wrong_cluster_logs_confusion/test_case.yaml, tests/llm/fixtures/test_ask_holmes/259_wrong_cluster_logs_confusion/toolsets.yaml
Adds a new wrong-cluster test: requires explicit connected-vs-requested cluster mismatch disclosure, sets up an app-259-prod-logs ES index and bulk-indexes 9 NDJSON log records in before_test, skips teardown, and restricts toolsets to Elasticsearch-only in toolsets.yaml.

Sequence Diagram(s)

sequenceDiagram
  participant User
  participant HolmesEngine
  participant Elasticsearch
  participant TestRunner
  User->>HolmesEngine: Ask about service/cluster
  HolmesEngine->>Elasticsearch: query connected index
  Elasticsearch-->>HolmesEngine: return logs (or none)
  HolmesEngine->>TestRunner: response (must state no-data or labeled adjacent findings / cluster mismatch)
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • HolmesGPT/holmesgpt#1801: Both PRs add/adjust Holmes Elasticsearch test fixtures to prevent cross-cluster log/diagnosis hallucinations by requiring explicit cluster mismatch handling and refusing root-cause claims based on logs from the wrong cluster.
  • HolmesGPT/holmesgpt#1711: Both PRs adjust Holmes prompt/instruction templates to change how answers should be worded when evidence is missing or uncertain.
  • HolmesGPT/holmesgpt#1858: Prior updates to the same Elasticsearch DR log-check fixture enforcing transparent handling for companyopswebjob vs similarly named services.

Suggested labels

evals-id-254

Suggested reviewers

  • moshemorad
  • arikalon1
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title directly and specifically describes the main change: improving how Holmes handles close-match naming behavior when exact entity names are not found.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ 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.

❤️ Share

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

@github-actions

github-actions Bot commented May 14, 2026 •

Copy link
Copy Markdown
Contributor

✅ Docker images ready for 49865345 (built in 4m 8s)

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

Use these tags to pull the images 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:49865345
docker tag us-central1-docker.pkg.dev/robusta-development/temporary-builds/holmes:49865345 me-west1-docker.pkg.dev/robusta-development/development/holmes-dev:49865345
docker push me-west1-docker.pkg.dev/robusta-development/development/holmes-dev:49865345
docker pull us-central1-docker.pkg.dev/robusta-development/temporary-builds/holmes-operator:49865345
docker tag us-central1-docker.pkg.dev/robusta-development/temporary-builds/holmes-operator:49865345 me-west1-docker.pkg.dev/robusta-development/development/holmes-operator-dev:49865345
docker push me-west1-docker.pkg.dev/robusta-development/development/holmes-operator-dev:49865345

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:49865345 \
  --set operator.registry=me-west1-docker.pkg.dev/robusta-development/development \
  --set operator.image=holmes-operator-dev:49865345

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:49865345 \
  --set holmes.operator.registry=me-west1-docker.pkg.dev/robusta-development/development \
  --set holmes.operator.image=holmes-operator-dev:49865345

@netlify

netlify Bot commented May 14, 2026 •

Copy link
Copy Markdown

✅ Deploy Preview for holmes-docs ready!

Name Link
🔨 Latest commit 5ecfea0
🔍 Latest deploy log https://app.netlify.com/projects/holmes-docs/deploys/6a21b8eeee22170008fadb08
😎 Deploy Preview https://deploy-preview-2042--holmes-docs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@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.

🧹 Nitpick comments (1)
tests/llm/fixtures/test_ask_holmes/254_elasticsearch_dr_test_log_check/test_case.yaml (1)

27-31: ⚡ Quick win

Consider whether prescriptive instructions reduce real-world test validity.

The added instructions explicitly tell Holmes how to filter (exact case-insensitive match), what not to do (don't infer from similar services), what to state (no logs found), and what to recommend (get logs from companyopswebjob). While this makes the test pass (per commit message), it raises a question: does this test diagnostic capability or instruction-following?

In real-world scenarios, users typically wouldn't provide such detailed filtering guidance. If Holmes needs this level of prescription to distinguish "companyopswebjob" from "Company.Ops" or "Company.Ops.Radar.WebJob", it might still fail when users simply ask "check logs for companyopswebjob" without explicit instructions.

The test may be more valuable if it verifies whether Holmes can naturally understand service boundaries rather than whether it can follow detailed filtering instructions. However, if the intent is specifically to test instruction-following for explicit filtering requirements, this approach is valid.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@tests/llm/fixtures/test_ask_holmes/254_elasticsearch_dr_test_log_check/test_case.yaml`
around lines 27 - 31, The test text currently enforces a highly prescriptive
filter line: "Important: only consider log entries whose service field matches
companyopswebjob exactly (case-insensitive)..." which biases the test toward
instruction-following rather than natural diagnostic ability; update
test_case.yaml to either (a) loosen that sentence to prefer an exact
case-insensitive match for the service field but allow the assistant to ask for
clarification or to explain ambiguity when similarly named services (e.g.,
Company.Ops, Company.Ops.Radar.WebJob) are present, or (b) split into two
assertions: one variant that enforces strict exact-match behavior and a second
variant that expects the assistant to infer service boundaries and ask
clarifying questions; reference the literal token "companyopswebjob" when
changing the YAML so the evaluator still targets the same service name.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In
`@tests/llm/fixtures/test_ask_holmes/254_elasticsearch_dr_test_log_check/test_case.yaml`:
- Around line 27-31: The test text currently enforces a highly prescriptive
filter line: "Important: only consider log entries whose service field matches
companyopswebjob exactly (case-insensitive)..." which biases the test toward
instruction-following rather than natural diagnostic ability; update
test_case.yaml to either (a) loosen that sentence to prefer an exact
case-insensitive match for the service field but allow the assistant to ask for
clarification or to explain ambiguity when similarly named services (e.g.,
Company.Ops, Company.Ops.Radar.WebJob) are present, or (b) split into two
assertions: one variant that enforces strict exact-match behavior and a second
variant that expects the assistant to infer service boundaries and ask
clarifying questions; reference the literal token "companyopswebjob" when
changing the YAML so the evaluator still targets the same service name.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 6db68c7f-d775-4868-bb9e-86dd793d331e

📥 Commits

Reviewing files that changed from the base of the PR and between bd0c5e0 and 57c9a58.

📒 Files selected for processing (1)
  • tests/llm/fixtures/test_ask_holmes/254_elasticsearch_dr_test_log_check/test_case.yaml

When the user asks about an entity that has no data, but a related/sibling
entity does, the desired behavior is to (1) state explicitly that no data
was found for the exact entity the user named, (2) still report what was
found in the similar-named entity, and (3) clearly label that the findings
come from a different but related entity.

This is more useful than refusing to share adjacent findings, and more
honest than silently substituting them for the user's named entity.

- generic_ask.jinja2: add a rule covering transparent-and-useful behavior
  for similarly-named entities, including an anti-hallucination clause
  forbidding silent merging like "service X (including <user's name>)"
  when the user's name is not in fact present.
- eval 254 (DR test log check): revert the prompt tweak from the previous
  commit; update expected_output to require both transparency (no-data
  for the exact name) and usefulness (report adjacent findings, clearly
  labeled).
- eval 19 (detect missing app details): same pattern — previously accepted
  EITHER behavior; now requires BOTH transparency about the missing name
  and useful reporting on the similar-named resource.

Verified by running eval 254 with 3 iterations on Sonnet 4.5, Opus 4.6,
and Opus 4.7 — 9/9 pass.

Signed-off-by: Claude <noreply@anthropic.com>

aantn commented May 14, 2026

Copy link
Copy Markdown
Collaborator Author

@coderabbitai you were right that the prescriptive instructions in the test prompt were biasing toward instruction-following — that change has been reverted in f90dcd2.

The fix is now on the Holmes side instead: holmes/plugins/prompts/generic_ask.jinja2 now instructs the model to be both transparent AND useful when it can't find the exact entity but finds similarly-named ones (state no data for the exact name, report what was found in the related entities, clearly label them as different services).

The test prompt is back to its original form. Expected_output now checks for both behaviors (transparency about the missing name + useful reporting on adjacent services, clearly labeled). 254 passes 9/9 across Sonnet 4.5, Opus 4.6, and Opus 4.7 with that change. Eval 19 was also updated from "either behavior" to "both behaviors" for consistency.


Generated by Claude Code

@coderabbitai

coderabbitai Bot commented May 14, 2026

Copy link
Copy Markdown
Contributor

Tip

For best results, initiate chat on the files or code changes.

@aantn That's a great outcome — fixing it at the generic_ask.jinja2 system-prompt level is the right call. The behavior is now genuinely part of the model's reasoning rather than a test-scoped instruction that could mask real gaps. Checking both transparency and usefulness (clearly labeled adjacent services) is also a stronger signal than the original "either/or" expected output. The 9/9 pass rate across all three model variants is a solid sign the change generalizes well.

(ʘᴗʘ✿)


✏️ Learnings added
Learnt from: aantn
URL: https://github.com/HolmesGPT/holmesgpt/pull/2042

Timestamp: 2026-05-14T21:14:34.667Z
Learning: In the HolmesGPT repo, when an LLM eval test case is failing because the model doesn't correctly filter by an exact entity name (e.g. service field), the right fix is to update the system-level prompt (e.g. `holmes/plugins/prompts/generic_ask.jinja2`) to instruct the model on the correct behavior — NOT to add prescriptive instructions directly into the test-case prompt, as that biases the eval toward instruction-following rather than testing genuine model behavior. The expected_output should check for both transparency (stating no data for the exact entity) and usefulness (reporting on similarly-named/adjacent entities, clearly labeled as different).

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

@github-actions

github-actions Bot commented May 14, 2026 •

Copy link
Copy Markdown
Contributor

🔬 CLI Performance Benchmark

🟡 Startup Time (no LLM)

Measures holmes version execution time (imports + initialization)

Metric PR Master Change
Cold Start 13.95s 14.27s -2.3%
Warm Mean 6.40s 6.70s -4.5%
Warm Min 6.21s 6.64s
Warm Max 6.61s 6.74s

🟡 Full CLI with LLM

Measures holmes ask execution time (OpenRouter + Haiku 4.5)

Metric PR Master Change
Cold Start 17.33s 14.85s +16.7%
Warm Mean 8.13s 8.85s -8.1%
Warm Min 8.02s 8.54s
Warm Max 8.25s 9.12s

PR: f6666bd9 | Master: 31fa24c1 | Iterations: 5

@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.

🧹 Nitpick comments (1)
holmes/plugins/prompts/generic_ask.jinja2 (1)

46-52: ⚡ Quick win

Consider clarifying the relationship between fuzzy-matching (line 46) and transparent adjacent reporting (lines 47-52).

Line 46 encourages assuming typos and searching for substrings/correct spellings when a resource isn't found. Lines 47-52 add guidance to explicitly state when the exact entity name has no data and then report similar entities with clear labels.

These instructions could work together (try fuzzy matching, then be transparent about what was found vs. requested), but the sequencing and priority aren't explicit. Consider adding a phrase like "After searching for variations (see above)," at the start of line 47, or clarifying whether the transparent reporting applies even when substring matches are found.

Suggested clarification
 * if you cannot find the resource/application that the user referred to, assume they made a typo or included/excluded characters like - and in this case, try to find substrings or search for the correct spellings
-* **Adjacent / similarly-named entities — be transparent AND useful:** when you cannot find data for the exact entity the user named, but you DO find data for one or more entities with similar names (sibling services, same prefix, same namespace, etc.), give the user both pieces of information:
+* **Adjacent / similarly-named entities — be transparent AND useful:** after searching for variations, when you cannot find data for the exact entity the user named, but you DO find data for one or more entities with similar names (sibling services, same prefix, same namespace, etc.), give the user both pieces of information:
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@holmes/plugins/prompts/generic_ask.jinja2` around lines 46 - 52, The guidance
around fuzzy-matching (the bullet at line 46) and the adjacent/similarly-named
reporting block (lines 47-52) is ambiguous about sequencing and priority; update
the adjacent-reporting section to explicitly state it runs after attempting
fuzzy/substring searches (e.g., begin that section with "After searching for
variations (see above),") and clarify that transparent reporting should still be
used whenever the exact requested entity has no data even if close matches were
found; reference the phrases "fuzzy-matching" / "search for substrings or
correct spellings" and the "Adjacent / similarly-named entities — be transparent
AND useful" block so reviewers can locate and apply the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@holmes/plugins/prompts/generic_ask.jinja2`:
- Around line 46-52: The guidance around fuzzy-matching (the bullet at line 46)
and the adjacent/similarly-named reporting block (lines 47-52) is ambiguous
about sequencing and priority; update the adjacent-reporting section to
explicitly state it runs after attempting fuzzy/substring searches (e.g., begin
that section with "After searching for variations (see above),") and clarify
that transparent reporting should still be used whenever the exact requested
entity has no data even if close matches were found; reference the phrases
"fuzzy-matching" / "search for substrings or correct spellings" and the
"Adjacent / similarly-named entities — be transparent AND useful" block so
reviewers can locate and apply the change.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 42f70cf5-246e-4403-837a-bd514f8ea58f

📥 Commits

Reviewing files that changed from the base of the PR and between 57c9a58 and f90dcd2.

📒 Files selected for processing (3)
  • holmes/plugins/prompts/generic_ask.jinja2
  • tests/llm/fixtures/test_ask_holmes/19_detect_missing_app_details/test_case.yaml
  • tests/llm/fixtures/test_ask_holmes/254_elasticsearch_dr_test_log_check/test_case.yaml

claude added 2 commits May 14, 2026 21:20
Per CodeRabbit nitpick on PR #2042: make explicit that the adjacent /
similarly-named entity rule applies after the fuzzy-match attempt, and
applies even when a close match exists.

Signed-off-by: Claude <noreply@anthropic.com>
Verified 9/9 pass across Sonnet 4.5, Opus 4.6, Opus 4.7 with the
generic_ask.jinja2 prompt fix from this PR.

Signed-off-by: Claude <noreply@anthropic.com>

@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.

🧹 Nitpick comments (1)
tests/llm/fixtures/test_ask_holmes/254_elasticsearch_dr_test_log_check/test_case.yaml (1)

14-16: ⚡ Quick win

Align expected_output with the stated “pull exact-service logs” expectation.

The scenario says the ideal response should recommend pulling companyopswebjob logs, but expected_output does not currently require that. Add a bullet so this behavior is actually enforced by the fixture.

Proposed patch
 expected_output:
   # Both behaviors are required — transparency AND usefulness.
@@
   # The two must be combined transparently — substitution is fine, hidden substitution is not.
   - "Must make it clear that any analysis or root-cause discussion is based on Company.Ops / Company.Ops.Radar.WebJob (related but different services), not on companyopswebjob's own logs. It is OK to analyze and answer based on the related services as long as this is disclosed."
+  - "Must recommend obtaining logs directly from companyopswebjob to confirm findings for the exact service the user requested."

Also applies to: 31-38

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@tests/llm/fixtures/test_ask_holmes/254_elasticsearch_dr_test_log_check/test_case.yaml`
around lines 14 - 16, Update the test fixture's expected_output to require the
assistant recommend pulling logs from "companyopswebjob": in the YAML
test_case.yaml add a bullet to the expected_output list that explicitly states
"pull logs from companyopswebjob" (or equivalent phrasing) so the scenario
enforces that behavior; apply the same change to the other expected_output
instance referenced (the block covering lines 31-38) so both fixtures require
the exact-service log pull for companyopswebjob.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In
`@tests/llm/fixtures/test_ask_holmes/254_elasticsearch_dr_test_log_check/test_case.yaml`:
- Around line 14-16: Update the test fixture's expected_output to require the
assistant recommend pulling logs from "companyopswebjob": in the YAML
test_case.yaml add a bullet to the expected_output list that explicitly states
"pull logs from companyopswebjob" (or equivalent phrasing) so the scenario
enforces that behavior; apply the same change to the other expected_output
instance referenced (the block covering lines 31-38) so both fixtures require
the exact-service log pull for companyopswebjob.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: f2b94954-06aa-4bb6-b1d0-140e38ce5e2a

📥 Commits

Reviewing files that changed from the base of the PR and between f90dcd2 and 0645ae3.

📒 Files selected for processing (2)
  • holmes/plugins/prompts/generic_ask.jinja2
  • tests/llm/fixtures/test_ask_holmes/254_elasticsearch_dr_test_log_check/test_case.yaml
🚧 Files skipped from review as they are similar to previous changes (1)
  • holmes/plugins/prompts/generic_ask.jinja2

Drop the 'should still pull logs from companyopswebjob to confirm'
phrasing from the description, since the expected_output deliberately
does not enforce that recommendation (CodeRabbit nitpick).

Signed-off-by: Claude <noreply@anthropic.com>

aantn commented May 14, 2026

Copy link
Copy Markdown
Collaborator Author

@coderabbitai re: the suggestion to add "Must recommend obtaining logs directly from companyopswebjob..." — that bullet was deliberately removed from expected_output earlier in this PR. The "ideal behavior" intentionally requires only (1) transparency about no exact-match data and (2) useful adjacent reporting clearly labeled — not a recommendation to fetch the named service's logs.

The header comment did still contain that phrasing, which is what created the inconsistency you spotted. I've removed it in b2282ab so the comment now matches the criteria.


Generated by Claude Code

@aantn aantn changed the title Add service field validation to Elasticsearch DR test case Improve 'close-matches' naming behaviour May 14, 2026
@coderabbitai

coderabbitai Bot commented May 14, 2026

Copy link
Copy Markdown
Contributor

Tip

For best results, initiate chat on the files or code changes.

@aantn Thanks for the clarification — that makes sense. The two-criteria design (transparency + labeled adjacent reporting) is deliberate, and the header comment was just stale. Good that b2282ab brings the comment in sync with the actual expected behavior.


If you found this review helpful, would you consider giving us a shout-out on X?

Thank you for using CodeRabbit!

Customers can install Holmes on multiple Kubernetes clusters / environments
simultaneously. A user can mistakenly ask the wrong Holmes agent about an
incident in a different cluster — this eval catches that.

Scenario: Holmes runs on production-eu-west-2 (set via cluster_name). The
user asks about an incident on production-us-east-2. The available logs are
plain app logs from the cluster Holmes is on, containing realistic checkout
500s, payment-provider timeouts, and a circuit-breaker trip — a very
compelling red herring that matches the user's described incident pattern.

Correct behavior: state upfront that this Holmes instance is connected to
production-eu-west-2, not production-us-east-2, so the data here belongs to
the wrong cluster. May still report the findings as adjacent context but
must label them as such; suggest the user verify they are on the correct
Holmes agent or provide us-east-2-specific data.

Verified RED on master (0/3 on both Sonnet 4.5 and Opus 4.7) and also RED
on this branch (0/3 on both). The current generic_ask.jinja2 does not
prompt Holmes to consider that it may be running on a different cluster
than the one the user is asking about; that is a separate follow-up.

Signed-off-by: Claude <noreply@anthropic.com>

@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: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In
`@tests/llm/fixtures/test_ask_holmes/259_wrong_cluster_logs_confusion/test_case.yaml`:
- Around line 68-70: Move the "set -e" invocation to the top of the script
before running or sourcing any setup commands so failures abort immediately;
specifically, ensure "set -e" appears before the "source
../../shared/es_test_utils.sh" line and before the call to "es_setup" so any
error in sourcing or in es_setup causes the test to fail fast.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 403ef6b7-f849-4edb-9eca-124394c7623c

📥 Commits

Reviewing files that changed from the base of the PR and between b2282ab and c1523da.

📒 Files selected for processing (2)
  • tests/llm/fixtures/test_ask_holmes/259_wrong_cluster_logs_confusion/test_case.yaml
  • tests/llm/fixtures/test_ask_holmes/259_wrong_cluster_logs_confusion/toolsets.yaml
✅ Files skipped from review due to trivial changes (1)
  • tests/llm/fixtures/test_ask_holmes/259_wrong_cluster_logs_confusion/toolsets.yaml

aantn commented May 15, 2026

Copy link
Copy Markdown
Collaborator Author

@coderabbitai re: moving set -e before source ../../shared/es_test_utils.sh / es_setup — skipping this in 259 because it would diverge from the convention used across all 21 existing ES-based test_case.yaml fixtures in tests/llm/fixtures/test_ask_holmes/ (e.g. 183a–g, 184–193, 235, 245, 254). They all place set -e after the source + es_setup calls. Worth doing as a separate cleanup across all 21 files rather than in this PR.


Generated by Claude Code

@coderabbitai

coderabbitai Bot commented May 15, 2026

Copy link
Copy Markdown
Contributor

Tip

For best results, initiate chat on the files or code changes.

@aantn That makes sense — consistency with the existing 21 fixtures is a good reason to defer. A bulk cleanup PR touching all of them at once is the cleaner approach.

Would you like me to open a follow-up GitHub issue to track the set -e placement cleanup across the ES-based test fixtures?


✏️ Learnings added
Learnt from: aantn
URL: https://github.com/HolmesGPT/holmesgpt/pull/2042

Timestamp: 2026-05-15T17:54:03.195Z
Learning: In the HolmesGPT repo (`tests/llm/fixtures/test_ask_holmes/`), all 21 existing Elasticsearch-based test_case.yaml `before_test` scripts place `set -e` *after* the `source ../../shared/es_test_utils.sh` and `es_setup` calls — not before them. This is the established convention; deviating from it in a single fixture is intentionally avoided, with the plan to do a bulk cleanup across all fixtures in a separate PR.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

aantn and others added 7 commits May 15, 2026 22:03
generic_ask.jinja2: extend the existing "You are running on cluster X"
bullet to tell Holmes that (a) a given Holmes agent has access only to
its own cluster, (b) customers commonly run separate Holmes agents per
cluster, and (c) if the user's question refers to a different cluster /
environment / region than the one Holmes is on, it MUST flag this
upfront before any root-cause discussion. Without this, Holmes would
silently treat its own cluster's data as if it were the user's named
cluster.

tests/llm/test_ask_holmes.py: pass test_case.cluster_name through to
build_initial_ask_messages in the CLI test path. Previously it was
silently dropped (only the server path applied it), so cluster_name in
test fixtures had no effect on the system prompt for ask-mode evals.

eval 259: now passes 3/3 on Sonnet 4.5 and Opus 4.6 and ~5/6 on Opus
4.7 with the cluster-awareness rule + cluster_name plumbing. Adding
the regression tag.

Signed-off-by: Claude <noreply@anthropic.com>
We don't use Braintrust datasets in this repo — all eval logging goes via
experiment span logs (log_to_braintrust → eval_span.log). The dataset
upload helpers have been dead code for a long time and the existing
records in the ask_holmes:master dataset are corrupted (input field is
literally the string '<built-in function input>' because of an
input=input bug where `input` was the Python builtin, not the
test_case's user_prompt).

Removed:
- tests/llm/utils/braintrust.py: BraintrustEvalHelper class (with
  upload_test_cases, resolve_dataset_item, start_evaluation,
  end_evaluation), find_dataset_row_by_test_case, pop_test_case,
  pop_matching_test_case_if_exists, get_dataset_name, the
  braintrust_enabled flag, and unused braintrust SDK imports
  (init_dataset / Dataset / Experiment / ReadonlyExperiment / Span /
  DummySpan).
- tests/llm/utils/langfuse.py: entire file, only contained an
  equivalent dataset upload path for Langfuse that nothing imports.
- dataset_record_id=test_case.id from eval_span.log calls in
  log_to_braintrust and test_holmes_checks.py. We don't link to a
  dataset anymore, and the active records already had this resolving
  to None at write time.

Signed-off-by: Claude <noreply@anthropic.com>
The "you are on cluster X, flag if user asks about a different one" rule
was buried ~13k characters deep in the system prompt — after the entire
TodoWrite mandatory-execution block — and Opus 4.6 was reliably ignoring
it on eval 259, blaming the local cluster's red-herring data as the root
cause of a different cluster's incident.

Hoisting the same rule into the intro block (right next to the
HolmesGPT version line, well before any other guidance) jumps eval 259
from 0/3 to 3/4 on Opus 4.6, 3/4 on Opus 4.7, and 4/4 on Sonnet 4.5
across 4 iterations each. The previous "# In general" copy of the rule
is now redundant and will be cleaned up if this version sticks.

Signed-off-by: Claude <noreply@anthropic.com>
Previously the cluster-awareness rule was both duplicated in the prompt
(once in the intro, once in "# In general") AND too blunt: it told Holmes
to refuse / flag whenever the user asked about a cluster other than its
local one. That's correct only when the agent is one-per-cluster. The
other common topology — a single Holmes wired to a global observability
backend (Elasticsearch / OpenSearch / Datadog / Loki / Prometheus with
multi-cluster scrape / etc.) — has legitimate cross-cluster data, and
the previous rule would make Holmes wrongly refuse to investigate.

This collapses the two duplicates into one rule and rewrites it to
distinguish:

- kubectl / in-cluster toolsets → only see {{ cluster_name }}.
- external observability toolsets → may contain data from many
  clusters; investigate, but VERIFY each finding's own cluster/region
  field (or index/source name) and label findings accordingly.

The required behavior when the user names a different cluster is now:

1. Investigate using external toolsets (do not refuse).
2. Check the data's own cluster/region/etc. fields.
3. If they match the user's cluster → investigate normally.
4. If they don't match → state plainly that you have no data for the
   user's cluster, label the adjacent data you DID find, and don't
   present it as the root cause.

Eval 260 (new) covers topology #2: Holmes is on production-eu-west-2,
user asks about production-us-east-2, the ES index has data for BOTH.
Holmes should investigate the us-east-2 data normally and not refuse.

Eval 259 setup updated so log entries carry a cluster/region field
(production-eu-west-2). Without that field there was no way for Holmes
to verify the data's origin, and it would reasonably proceed; with the
field, Holmes can apply step 4 above.

Local pass rate across 3 iterations × 3 models:
  259 (no remote data, must flag): 7/9 (Opus 4.6 3/3, Opus 4.7 1/3,
                                       Sonnet 4.5 3/3)
  260 (global data, must investigate): 9/9

Signed-off-by: Claude <noreply@anthropic.com>
Continuing the cluster-awareness work in 259/260, fill in the missing
edge cases for multi-cluster / multi-env / multi-source topologies and
generic name-disambiguation.

ES-based (validated locally — 28/28 across 2 iterations on Opus 4.7 +
Sonnet 4.5):

  261_time_window_gap_external_data
      ES index has data for the user's cluster but only the last ~30
      minutes; user asks about a 6-hours-ago incident. Holmes must
      flag the time-window gap, not invent an answer from recent
      INFO logs.

  262_ambiguous_cluster_reference
      cluster_name is the generic "production"; user says "the US
      prod cluster". Holmes must not silently treat its local
      cluster as the one the user meant — must clarify or
      explicitly investigate production-us-east-1 with labels.

  263_region_suffixed_service_siblings
      User says "payment-service is failing"; index has
      payment-service-us, -eu, -ap. Holmes must surface the
      ambiguity and attribute findings to the specific variant.

  264_wrong_env_same_region
      Holmes on staging-eu-west-2, user asks about
      production-eu-west-2. Same region, different env — easy to
      miss. Holmes must catch the env mismatch.

  265_multicluster_labeling_discipline
      Open-ended "what's broken across our fleet?" with multi-cluster
      ES data. Every finding in the answer must carry the cluster
      it came from — generic "checkout-service is failing" without
      a cluster is a fail.

  266_toolset_disabled_vs_no_data
      User asks Holmes to check Datadog APM. Datadog toolset is
      disabled. Holmes must say it can't access Datadog at all,
      not claim it looked and found nothing.

  267_cluster_name_alias
      User refers to the local cluster by a casual alias ("EU prod"
      for "acme-prod-eu-west-1"). Holmes must recognise the
      equivalence and investigate normally — must NOT flag a
      mismatch and refuse. Counter-test to the cluster-awareness
      rule to prevent over-correction.

Kubectl-based (fixtures written, validation deferred to CI — no KIND
locally):

  268_kubectl_wrong_cluster
      No external toolsets; only kubectl (local cluster only). User
      asks about a different cluster. Holmes must flag kubectl-local
      limitation, not search and present results from the local
      cluster as the answer.

  269_mixed_sources_route_to_external
      kubectl (local only) + Elasticsearch (multi-cluster). User
      asks about a non-local cluster. Holmes must route to ES, not
      kubectl. Local healthy state must not be presented as the
      answer about a different cluster.

  270_namespace_collision
      Same-named deployment in two namespaces with different
      states (one crashlooping, one healthy). User asks "why is
      checkout-service down?" without a namespace. Holmes must
      surface the ambiguity and address both, not silently pick
      one.

Signed-off-by: Claude <noreply@anthropic.com>
claude added 2 commits June 3, 2026 07:20
So you can run the whole set with:
  poetry run pytest -m multi-cluster --no-cov
  # or in CI:
  /eval
  tags: multi-cluster

13 evals tagged: 254, 259, 260, 261-270. New marker registered in
pyproject.toml.

Signed-off-by: Claude <noreply@anthropic.com>
aantn pushed a commit that referenced this pull request Jun 4, 2026
Address CodeRabbit nits on the new multi-cluster eval fixtures:

- 259: fix echo describing data as having no cluster/region fields when
  the documents do include them.
- 261, 269: validate the Elasticsearch _bulk response and exit non-zero
  on partial failures, matching the pattern already used in 254.
- 269, 270: fail-fast when pod-readiness retry loops exhaust without
  reaching the expected state, per the CLAUDE.md race-condition pattern.
- 265: drop the unverifiable "/ 500s" suffix from expected_output - the
  fixture logs do not contain any 500 status code, so the eval
  criterion should not reference one.

No test-semantics change; this is fixture robustness only. The combined
PR #2042 (evals + prompt fix) already runs 24/24 green with these
fixtures; these tweaks reduce the chance of misleading failures from
silent setup drift.

https://claude.ai/code/session_015scgyQbqJD23heqct31Ger
Signed-off-by: Claude <noreply@anthropic.com>
aantn added a commit that referenced this pull request Jun 7, 2026
Splits PR #2042 into two: this is the **red** half. Tests/evals +
supporting cleanup, **no prompt change**.

Without the companion green PR (#TBD —
`claude/multi-cluster-prompt-green`), the new evals are expected to be
RED on master. They demonstrate the desired behavior; the prompt change
in the green PR turns them green.

## What this PR contains

### 13 evals (all tagged `multi-cluster`)

| # | Test | Scenario |
|---|---|---|
| 254 (updated) | `elasticsearch_dr_test_log_check` | Similar-name
service disambiguation (`companyopswebjob` vs `Company.Ops*`). |
| 19 (updated) | `detect_missing_app_details` |
`personal-certs-validator` vs `db-certs-authenticator` — transparency +
usefulness both required. |
| 259 | `wrong_cluster_logs_confusion` | Holmes on
`production-eu-west-2`, user asks about `production-us-east-2`; ES has
only local data with red-herring 500s. |
| 260 | `global_es_remote_cluster_logs` | Same shape, but ES has
remote-cluster data (global topology) — must investigate normally. |
| 261 | `time_window_gap_external_data` | Data exists but only recent;
user asks about older incident. |
| 262 | `ambiguous_cluster_reference` | `cluster_name="production"`;
user says "the US prod cluster". |
| 263 | `region_suffixed_service_siblings` |
`payment-service-us/-eu/-ap`; user says "payment-service". |
| 264 | `wrong_env_same_region` | Holmes on `staging-eu-west-2`; user
asks about `production-eu-west-2`. |
| 265 | `multicluster_labeling_discipline` | Open-ended fleet question;
every finding must be cluster-attributed. |
| 266 | `toolset_disabled_vs_no_data` | Datadog APM requested, toolset
disabled — must distinguish from "found nothing". |
| 267 | `cluster_name_alias` | User says "EU prod" for
`acme-prod-eu-west-1` — must NOT over-correct. |
| 268 (KIND) | `kubectl_wrong_cluster` | kubectl-only path; non-local
cluster asked about. |
| 269 (KIND+ES) | `mixed_sources_route_to_external` | Must route to
external backend for non-local cluster. |
| 270 (KIND) | `namespace_collision` | Same deployment name in 2
namespaces. |

Run the whole set with:
```
/eval
tags: multi-cluster
```

### Supporting changes

- **`tests/llm/test_ask_holmes.py`**: pass `test_case.cluster_name`
through to `build_initial_ask_messages` in the CLI test path. Previously
it was silently dropped, so `cluster_name` in test fixtures had no
effect on the system prompt for ask-mode evals.
- **`tests/llm/utils/braintrust.py` + `tests/llm/utils/langfuse.py`**:
purge legacy dataset-upload code (`BraintrustEvalHelper`,
`upload_test_cases`, `find_dataset_row_by_test_case`, etc.). We don't
use Braintrust datasets in this repo — all eval logging goes via
experiment span logs. The legacy code also had a real bug (`input=input`
where `input` was the Python builtin) that corrupted dataset records.
- **`tests/llm/test_holmes_checks.py`**: drop `dataset_record_id=` from
`eval_span.log` calls (we no longer link to a dataset).
- **`pyproject.toml`**: register the new `multi-cluster` pytest marker.

### Companion PR

Merge AFTER the green PR (`claude/multi-cluster-prompt-green`), which
adds the cluster-awareness rule to
`holmes/plugins/prompts/generic_ask.jinja2`. Order doesn't matter for
the git merge, but red-then-green will show the red→green transition in
CI; green-first will show no red phase.

---
_Generated by [Claude
Code](https://claude.ai/code/session_015scgyQbqJD23heqct31Ger)_

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

* **Tests**
* Added and enhanced 13+ LLM test scenarios for multi-cluster,
environment, service-ambiguity and routing cases; tightened expectations
for transparent, explicit responses when data or resources are missing
or from a different cluster.
* **Chores**
  * Added a pytest marker for multi-cluster evaluations.
* Removed Braintrust and Langfuse dataset utilities and simplified
evaluation logging.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Signed-off-by: Claude <noreply@anthropic.com>
Co-authored-by: Claude <noreply@anthropic.com>
@aantn aantn closed this Jun 7, 2026
aantn added a commit that referenced this pull request Jun 7, 2026
)

Splits PR #2042 into two — this is the **green** half. Just the
system-prompt change. Pair with #2125 (red) which contains the evals.

## What this PR changes

`holmes/plugins/prompts/generic_ask.jinja2`: expands the existing `* You
are running on cluster X` bullet into a multi-cluster-aware procedure.

**Old:**
```jinja2
{% if cluster_name -%}
* You are running on cluster {{ cluster_name }}.
{%- endif %}
```

**New:** distinguishes kubectl-bound data (only the local cluster) from
external observability toolsets (Elasticsearch / Datadog / Loki / etc.,
which may contain data from many clusters). When the user names a
cluster / region / env other than the local one, Holmes:

1. Investigates using external toolsets — does NOT refuse.
2. Verifies each finding's cluster against the user's named cluster (via
the data's own `cluster` / `region` / `environment` /
`kubernetes.cluster.name` field, or the index/source name).
3. If matched → investigates normally with that cluster's findings.
4. If not matched → states plainly that no data exists for the requested
cluster, labels what was found in adjacent clusters, suggests pointing
Holmes at the right data source / agent.

Two common topologies are explicitly called out:
- one Holmes per cluster (kubectl + external mostly scoped to that
cluster)
- one Holmes with a global observability backend covering many clusters

Holmes doesn't know upfront which one applies — it must look at what the
data actually contains.

## Companion PR

This turns #2125's evals green:

- 254, 19 — name disambiguation
- 259 — wrong cluster, local-only data
- 260 — global topology, remote cluster data
- 261 — time-window gap
- 262 — ambiguous cluster reference
- 263 — region-suffixed sibling services
- 264 — wrong env, same region
- 265 — labeling discipline
- 266 — toolset-disabled vs no-data
- 267 — cluster-name alias (must NOT over-correct)
- 268/269/270 — kubectl-only / mixed-source / namespace-collision

Local validation across 3 iterations × 3 models (Sonnet 4.5 / Opus 4.6 /
Opus 4.7) on the ES-based subset was clean. CI on #2125 will demonstrate
the failing state; merging this on top turns those failures green.

---
_Generated by [Claude
Code](https://claude.ai/code/session_015scgyQbqJD23heqct31Ger)_

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

## Release Notes

* **Improvements**
* Enhanced cluster awareness to ensure observability data is correctly
attributed to your specified cluster and prevent misinterpretation of
cross-cluster data.
* Improved transparency when exact entity data is unavailable, with
clearer reporting of similarly-named alternatives for user verification.
* Refined troubleshooting guidance to better support iterative
investigation workflows.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->

Signed-off-by: Claude <noreply@anthropic.com>
Co-authored-by: Claude <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants