Revert moving Kubernetes instructions from system prompt to skill and other improvements to evals - #2051
Merged
Conversation
Adds eval 259_loki_historical_logs_pod_deleted_docker, a Kubernetes-free sibling of 101_loki_historical_logs_pod_deleted that runs Loki in a plain Docker container and pushes the historical log timeline via the Loki HTTP API. Lets developers reproduce the regression locally in environments where kind/k3s won't start (e.g. cgroup-v1 sandboxes). Verified end-to-end with the existing OpenRouter recipe (1/1 pass, 92.4s, 25 tool calls, $0.47). Also fixes the classifier when MODEL_LIST_FILE_LOCATION pins a model with the "openrouter/" litellm prefix. autoevals talks to the openai SDK directly (not litellm), so the prefix has to be stripped before the request; previously the call went out as model=openrouter/openai/gpt-4.1 and either hit api.openai.com with an OpenRouter key or got rejected by OpenRouter as an invalid model id, depending on how OPENAI_BASE_URL was set. Signed-off-by: Claude <noreply@anthropic.com>
5 iters of eval 259_loki_historical_logs_pod_deleted_docker on each side, opus-4.6 via OpenRouter: baseline cf6ddb7 (2026-04-30): 78.6s $0.407 7.8 turns 21.8 tools 194K tk current 1b61fe3 (2026-05-15): 89.4s $0.447 10.6 turns 22.0 tools 259K tk +13.7% +9.8% +35.9% +0.9% +33.6% Pass rate unchanged (5/5 vs 5/5). Tool count flat. Output tokens flat. Total/cached input tokens up ~33-41% (z > 3.8, highly significant). Turns up ~36% (z > 3) — same tool work, spread over more serialized rounds. That's the signature of a bigger system prompt driving more incremental multi-step exploration, matching the PR-#1970 hypothesis from the earlier single-iter ci-benchmark vs master-CI comparison. Includes the run_sweep.sh helper used to produce these numbers. Signed-off-by: Claude <noreply@anthropic.com>
Pulled raw spans from Braintrust for test 101_loki_historical_logs_pod_deleted on opus-4.6 — baseline run is ci-benchmark-25268492423 (cf6ddb7, May 3) and current is master-25938985537 (1b61fe3, May 15). Per-call prompt sizes are essentially identical (call 1: 16,826 vs 16,784). The regression is one extra LLM call that re-sends the ~27K cached prefix. System prompt itself is 3.2% SMALLER on current (43,248 -> 41,863 chars). What actually changed in the prompt: - "Whenever possible you MUST first use tools" — REMOVED - New "Skill Usage" preamble added; Phase 1 now checks skills before TodoWrite - The whole "If investigating Kubernetes problems" section (14 lines of concrete kubectl guidance) was deleted and externalised as the kubernetes-troubleshooting skill (PR #2040) - User prompt grew +116% (1,008 -> 2,176 chars) with a new "# Skill Selection" catalog that names kubernetes-troubleshooting and tells the model to fetch it before investigating Net effect: same task gets investigated in more, smaller rounds. The model deliberates, fetches a skill, then investigates — instead of just investigating. On the docker-loki sweep where kubectl is unavailable, the skill fetch is wasted work and turns climb +36%. Includes the raw baseline/current system+user prompt files and the unified diff, so reviewers can inspect the change themselves. Signed-off-by: Claude <noreply@anthropic.com>
Tested each suggested fix in isolation, 5 iters of the docker-loki eval
each, opus-4.6 via OpenRouter. All 20 runs passed correctness.
Means (n=5):
baseline current fix-B fix-A
time (s) 78.6 89.4 89.9 87.4
turns 7.8 10.6 9.6 9.2
total tokens 194K 259K 237K 231K
cached input 156K 220K 198K 192K
cost ($) 0.41 0.45 0.44 0.43
fetch_skill log lines (Σ5) 0 29 19 10
- fix-B (restore "Whenever possible you MUST first use tools" one-liner):
cuts turns/tokens ~10%, no wall-clock improvement.
- fix-A (restore the deleted "If investigating Kubernetes problems"
section): cuts turns/tokens ~10–15%, cuts skill-fetch chatter ~65%.
Neither alone closes the gap. Both help in roughly the same direction.
Likely cause for the residual gap: the user-prompt "# Skill Selection"
block added by PR #2040 still names kubernetes-troubleshooting, so the
model still considers fetching the skill once per session even when the
inline guidance is back.
Suggested follow-up: combine fix-A + fix-B and gate the user-prompt
skill catalog block on actual skill relevance.
Includes raw 5×2 = 10 iter reports for both fix variants and the
side-by-side analysis in fix-comparison.md.
Signed-off-by: Claude <noreply@anthropic.com>
n=5 sweep across six candidate fixes pinpointed PR #2040 as the cause. Fix-AD (full revert of PR #2040: restore the deleted '# If investigating Kubernetes problems' section in generic_ask.jinja2 AND delete holmes/plugins/skills/builtin/kubernetes-troubleshooting/) restores baseline behavior: metric baseline current fix-AD recovery time 78.6s 89.4s 83.0s 0.59 turns 7.8 10.6 8.6 0.71 total tokens 194K 259K 203K 0.86 cached 156K 220K 166K 0.85 cost $0.41 $0.45 $0.40 fully (slightly cheaper than baseline) fetch_skill (Σ5) 0 29 0 — z-score vs baseline drops from 5.11 (turns) on current to 1.79 on fix-AD — i.e. inside the noise floor. Per-iter spread on fix-AD sits entirely inside the baseline range, no outliers. Per-iter inspection showed the smoking gun: every post-#2040 iter called fetch_skill exactly once on turn 1 (pulling the kubernetes-troubleshooting skill). Baseline had zero such calls because the skill simply didn't exist. The user-prompt skill catalog block was identical between baseline and current, but only renders content when a skill is loaded — PR #2040 was what loaded one. Fix-A alone (prompt section only) recovered ~45% because the skill file still existed and still got listed. Fix-B (restoring the 'MUST use tools' one-liner from PR #1970) didn't help at all. Fix-AB and fix-AC were equivalent to fix-A. Only fix-AD closes the gap. The recommended PR diff is in analysis/2026-05-15-regression/the-fix.md. Raw 5-iter reports for all six conditions are committed alongside. Signed-off-by: Claude <noreply@anthropic.com>
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
Contributor
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.
Contributor
📂 Previous Runs📜 #2 · Run @ __00a888e__ (#25965971118) — May 16, 15:47 UTC✅ Results of HolmesGPT evalsAutomatically triggered by commit 00a888e on branch Results of HolmesGPT evals
Benchmark Comparison DetailsMaster baseline: latest master-* experiment (post-merge regression eval)
Benchmark baseline: latest ci-benchmark experiment on master
Time comparison (seconds):
Cost comparison:
Total tokens comparison:
Cached tokens comparison:
Turns comparison:
Tool calls comparison:
Comparison indicators:
|
| Status | Test case | Time | Turns | Tools | Cost | Total tokens | Input | Max input | Output | Max output | Cached | Non-cached | Reasoning | Compactions | Src |
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| ✅ | 09_crashpod | 39.5s | 6 | 12 | $0.2981 | 131,561 | 129,027 | 24,435 | 2,534 | 1,060 | 104,012 | 25,015 | 294 | — | src |
| ✅ | 101_loki_historical_logs_pod_deleted | 81.4s | 8 | 19 | $0.4834 | 210,796 | 205,276 | 31,620 | 5,520 | 1,001 | 169,870 | 35,406 | 1,414 | — | src |
| ✅ | 112_find_pvcs_by_uuid | 19.2s | 3 | 4 | $0.2041 | 60,784 | 59,602 | 21,747 | 1,182 | 613 | 37,597 | 22,005 | 292 | — | src |
| ✅ | 12_job_crashing | 43.8s | 6 | 14 | $0.3172 | 135,211 | 132,402 | 25,633 | 2,809 | 606 | 105,768 | 26,634 | 203 | — | src |
| ✅ | 176_network_policy_blocking_traffic_no_skills | 51.2s | 6 | 14 | $0.3588 | 143,732 | 140,479 | 28,138 | 3,253 | 749 | 109,484 | 30,995 | 647 | — | src |
| ✅ | 227_count_configmaps_per_namespace[0] | 19.5s | 4 | 9 | $0.2068 | 76,266 | 75,140 | 20,545 | 1,126 | 594 | 53,669 | 21,471 | 53 | — | src |
| ✅ | 243_pod_names_contain_service | 35.6s | 5 | 9 | $0.2600 | 101,960 | 99,839 | 22,423 | 2,121 | 908 | 76,417 | 23,422 | 261 | — | src |
| ✅ | 24_misconfigured_pvc | 45.4s | 6 | 15 | $0.3314 | 135,135 | 131,974 | 25,492 | 3,161 | 1,132 | 104,340 | 27,634 | 534 | — | src |
| ✅ | 43_current_datetime_from_prompt | 3.6s | 1 | — | $0.1183 | 16,900 | 16,798 | 16,798 | 102 | 102 | 0 | 16,798 | 61 | — | src |
| ✅ | 51_logs_summarize_errors | 20.4s | 4 | 5 | $0.2012 | 76,150 | 75,056 | 20,529 | 1,094 | 383 | 54,522 | 20,534 | 73 | — | src |
| ✅ | 61_exact_match_counting | 10.2s | 3 | 3 | $0.1511 | 52,426 | 52,063 | 17,775 | 363 | 216 | 34,284 | 17,779 | 32 | — | src |
| Total | 33.6s avg | 4.7 avg | 10.4 avg | $2.9304 | 1,140,921 | 1,117,656 | 31,620 | 23,265 | 1,132 | 849,963 | 267,693 | 3,864 | — |
Benchmark Comparison Details
Master baseline: latest master-* experiment (post-merge regression eval)
Status: 11 test/model combinations loaded
- master-25943210368 (created: 2026-05-15)
Benchmark baseline: latest ci-benchmark experiment on master
Status: 147 test/model combinations loaded
- ci-benchmark-25268492423 (created: 2026-05-03)
Time comparison (seconds):
| Test case | This branch | master (18h ago) | Δ vs master | benchmark (13d ago) | Δ vs benchmark |
|---|---|---|---|---|---|
| 09_crashpod (opus-4.6) 📄 | 39.5s | — | — | 32.5s | ↑22% |
| 101_loki_historical_logs_pod_deleted (opus-4.6) 📄 | 81.4s | 81.2s | ±0% | 51.7s | ↑58% |
| 112_find_pvcs_by_uuid (opus-4.6) 📄 | 19.2s | 18.5s | ±0% | 18.1s | ±0% |
| 12_job_crashing (opus-4.6) 📄 | 43.8s | 41.4s | ±0% | 42.4s | ±0% |
| 176_network_policy_blocking_traffic_no_skills (opus-4.6) 📄 | 51.2s | 43.1s | ↑19% | 35.7s | ↑43% |
| 227_count_configmaps_per_namespace[0] (opus-4.6) 📄 | 19.5s | 19.3s | ±0% | 19.7s | ±0% |
| 243_pod_names_contain_service (opus-4.6) 📄 | 35.6s | 34.1s | ±0% | 27.4s | ↑30% |
| 24_misconfigured_pvc (opus-4.6) 📄 | 45.4s | 34.3s | ↑32% | 35.8s | ↑27% |
| 43_current_datetime_from_prompt (opus-4.6) 📄 | 3.6s | 4.1s | ↓12% | — | — |
| 51_logs_summarize_errors (opus-4.6) 📄 | 20.4s | 22.9s | ↓11% | 23.1s | ↓11% |
| 61_exact_match_counting (opus-4.6) 📄 | 10.2s | 9.5s | ±0% | 10.8s | ±0% |
| Total (all, n=11) | 33.6s | 30.9s | — | 29.7s | — |
| Comparable (m=10, b=10) | 33.0s | 30.9s | ±0% | 29.7s | ↑23% |
Cost comparison:
| Test case | This branch | master (18h ago) | Δ vs master | benchmark (13d ago) | Δ vs benchmark |
|---|---|---|---|---|---|
| 09_crashpod (opus-4.6) 📄 | $0.2981 | $0.2716 | ±0% | $0.2616 | ↑14% |
| 101_loki_historical_logs_pod_deleted (opus-4.6) 📄 | $0.4834 | $0.4857 | ±0% | $0.3371 | ↑43% |
| 112_find_pvcs_by_uuid (opus-4.6) 📄 | $0.2041 | $0.2053 | ±0% | $0.2014 | ±0% |
| 12_job_crashing (opus-4.6) 📄 | $0.3172 | $0.3388 | ±0% | $0.3076 | ±0% |
| 176_network_policy_blocking_traffic_no_skills (opus-4.6) 📄 | $0.3588 | $0.2944 | ↑22% | $0.2914 | ↑23% |
| 227_count_configmaps_per_namespace[0] (opus-4.6) 📄 | $0.2068 | $0.2035 | ±0% | $0.2059 | ±0% |
| 243_pod_names_contain_service (opus-4.6) 📄 | $0.2600 | $0.2539 | ±0% | $0.2280 | ↑14% |
| 24_misconfigured_pvc (opus-4.6) 📄 | $0.3314 | $0.2656 | ↑25% | $0.2831 | ↑17% |
| 43_current_datetime_from_prompt (opus-4.6) 📄 | $0.1183 | $0.1202 | ±0% | — | — |
| 51_logs_summarize_errors (opus-4.6) 📄 | $0.2012 | $0.2109 | ±0% | $0.2072 | ±0% |
| 61_exact_match_counting (opus-4.6) 📄 | $0.1511 | $0.1522 | ±0% | $0.1522 | ±0% |
| Total (all, n=11) | $0.2664 | $0.2547 | — | $0.2475 | — |
| Comparable (m=11, b=10) | $0.2664 | $0.2547 | ±0% | $0.2475 | ↑14% |
Total tokens comparison:
| Test case | This branch | master (18h ago) | Δ vs master | benchmark (13d ago) | Δ vs benchmark |
|---|---|---|---|---|---|
| 09_crashpod (opus-4.6) 📄 | 131,561 | 105,496 | ↑25% | 103,497 | ↑27% |
| 101_loki_historical_logs_pod_deleted (opus-4.6) 📄 | 210,796 | 211,614 | ±0% | 138,670 | ↑52% |
| 112_find_pvcs_by_uuid (opus-4.6) 📄 | 60,784 | 61,202 | ±0% | 61,169 | ±0% |
| 12_job_crashing (opus-4.6) 📄 | 135,211 | 150,381 | ↓10% | 133,893 | ±0% |
| 176_network_policy_blocking_traffic_no_skills (opus-4.6) 📄 | 143,732 | 108,107 | ↑33% | 111,145 | ↑29% |
| 227_count_configmaps_per_namespace[0] (opus-4.6) 📄 | 76,266 | 76,859 | ±0% | 76,945 | ±0% |
| 243_pod_names_contain_service (opus-4.6) 📄 | 101,960 | 82,320 | ↑24% | 79,525 | ↑28% |
| 24_misconfigured_pvc (opus-4.6) 📄 | 135,135 | 103,148 | ↑31% | 108,047 | ↑25% |
| 43_current_datetime_from_prompt (opus-4.6) 📄 | 16,900 | 17,075 | ±0% | — | — |
| 51_logs_summarize_errors (opus-4.6) 📄 | 76,150 | 78,273 | ±0% | 77,707 | ±0% |
| 61_exact_match_counting (opus-4.6) 📄 | 52,426 | 52,847 | ±0% | 52,942 | ±0% |
| Total (all, n=11) | 103,720 | 95,211 | — | 94,354 | — |
| Comparable (m=11, b=10) | 103,720 | 95,211 | ±0% | 94,354 | ↑19% |
Cached tokens comparison:
| Test case | This branch | master (18h ago) | Δ vs master | benchmark (13d ago) | Δ vs benchmark |
|---|---|---|---|---|---|
| 09_crashpod (opus-4.6) 📄 | 104,012 | 78,926 | ↑32% | 77,391 | ↑34% |
| 101_loki_historical_logs_pod_deleted (opus-4.6) 📄 | 169,870 | 169,448 | ±0% | 106,565 | ↑59% |
| 112_find_pvcs_by_uuid (opus-4.6) 📄 | 37,597 | 37,869 | ±0% | 38,002 | ±0% |
| 12_job_crashing (opus-4.6) 📄 | 105,768 | 118,280 | ↓11% | 104,761 | ±0% |
| 176_network_policy_blocking_traffic_no_skills (opus-4.6) 📄 | 109,484 | 79,686 | ↑37% | 81,519 | ↑34% |
| 227_count_configmaps_per_namespace[0] (opus-4.6) 📄 | 53,669 | 55,029 | ±0% | 54,503 | ±0% |
| 243_pod_names_contain_service (opus-4.6) 📄 | 76,417 | 56,295 | ↑36% | 55,513 | ↑38% |
| 24_misconfigured_pvc (opus-4.6) 📄 | 104,340 | 76,928 | ↑36% | 80,270 | ↑30% |
| 43_current_datetime_from_prompt (opus-4.6) 📄 | — | — | — | — | — |
| 51_logs_summarize_errors (opus-4.6) 📄 | 54,522 | 55,680 | ±0% | 55,443 | ±0% |
| 61_exact_match_counting (opus-4.6) 📄 | 34,284 | 34,567 | ±0% | 34,632 | ±0% |
| Total (all, n=11) | 77,269 | 69,337 | — | 68,860 | — |
| Comparable (m=10, b=10) | 84,996 | 76,271 | ↑11% | 68,860 | ↑23% |
Turns comparison:
| Test case | This branch | master (18h ago) | Δ vs master | benchmark (13d ago) | Δ vs benchmark |
|---|---|---|---|---|---|
| 09_crashpod (opus-4.6) 📄 | 6 | 5 | ↑20% | — | — |
| 101_loki_historical_logs_pod_deleted (opus-4.6) 📄 | 8 | 8 | ±0% | — | — |
| 112_find_pvcs_by_uuid (opus-4.6) 📄 | 3 | 3 | ±0% | — | — |
| 12_job_crashing (opus-4.6) 📄 | 6 | 6 | ±0% | — | — |
| 176_network_policy_blocking_traffic_no_skills (opus-4.6) 📄 | 6 | 5 | ↑20% | — | — |
| 227_count_configmaps_per_namespace[0] (opus-4.6) 📄 | 4 | 4 | ±0% | — | — |
| 243_pod_names_contain_service (opus-4.6) 📄 | 5 | 4 | ↑25% | — | — |
| 24_misconfigured_pvc (opus-4.6) 📄 | 6 | 5 | ↑20% | — | — |
| 43_current_datetime_from_prompt (opus-4.6) 📄 | 1 | 1 | ±0% | — | — |
| 51_logs_summarize_errors (opus-4.6) 📄 | 4 | 4 | ±0% | — | — |
| 61_exact_match_counting (opus-4.6) 📄 | 3 | 3 | ±0% | — | — |
| Total (all, n=11) | 4.7 | 4.4 | — | — | — |
| Comparable (m=11, b=0) | 4.7 | 4.4 | ±0% | — | — |
Tool calls comparison:
| Test case | This branch | master (18h ago) | Δ vs master | benchmark (13d ago) | Δ vs benchmark |
|---|---|---|---|---|---|
| 09_crashpod (opus-4.6) 📄 | 12 | 10 | ↑20% | 10 | ↑20% |
| 101_loki_historical_logs_pod_deleted (opus-4.6) 📄 | 19 | 22 | ↓14% | 14 | ↑36% |
| 112_find_pvcs_by_uuid (opus-4.6) 📄 | 4 | 4 | ±0% | 4 | ±0% |
| 12_job_crashing (opus-4.6) 📄 | 14 | 13 | ±0% | 14 | ±0% |
| 176_network_policy_blocking_traffic_no_skills (opus-4.6) 📄 | 14 | 12 | ↑17% | 13 | ±0% |
| 227_count_configmaps_per_namespace[0] (opus-4.6) 📄 | 9 | 9 | ±0% | 9 | ±0% |
| 243_pod_names_contain_service (opus-4.6) 📄 | 9 | 10 | ↓10% | 8 | ↑12% |
| 24_misconfigured_pvc (opus-4.6) 📄 | 15 | 11 | ↑36% | 14 | ±0% |
| 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=11) | 9.5 | 9.9 | — | 9.4 | — |
| Comparable (m=10, b=10) | 10.4 | 9.9 | ±0% | 9.4 | ↑11% |
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:/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/investigate-evals-regression-EYQQC -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, 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/investigate-evals-regression-EYQQC -f markers=regression -f filter=
Contributor
|
Note Reviews pausedIt 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 Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds a Docker-based Loki test fixture with a log generator and orchestration, inserts Kubernetes troubleshooting guidance into the generic prompt, detects OpenRouter-prefixed classifier models in the model-list path, and adds eval-regression diagnostic documentation. ChangesLoki Docker Test Fixture for Pod Deleted Scenario
Kubernetes Troubleshooting Guidance Enhancement
OpenRouter Classifier Model Support
Eval Regression Diagnostic Documentation
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
Contributor
|
✅ 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:b35d71ea
docker tag us-central1-docker.pkg.dev/robusta-development/temporary-builds/holmes:b35d71ea me-west1-docker.pkg.dev/robusta-development/development/holmes-dev:b35d71ea
docker push me-west1-docker.pkg.dev/robusta-development/development/holmes-dev:b35d71ea
docker pull us-central1-docker.pkg.dev/robusta-development/temporary-builds/holmes-operator:b35d71ea
docker tag us-central1-docker.pkg.dev/robusta-development/temporary-builds/holmes-operator:b35d71ea me-west1-docker.pkg.dev/robusta-development/development/holmes-operator-dev:b35d71ea
docker push me-west1-docker.pkg.dev/robusta-development/development/holmes-operator-dev:b35d71eaPatch 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:b35d71ea \
--set operator.registry=me-west1-docker.pkg.dev/robusta-development/development \
--set operator.image=holmes-operator-dev:b35d71eaRobusta 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:b35d71ea \
--set holmes.operator.registry=me-west1-docker.pkg.dev/robusta-development/development \
--set holmes.operator.image=holmes-operator-dev:b35d71ea |
✅ Deploy Preview for holmes-docs ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
- Restore the "# If investigating Kubernetes problems" section to generic_ask.jinja2 that was removed in PR #2040 - Delete holmes/plugins/skills/builtin/kubernetes-troubleshooting/ so the skill no longer registers in the catalog - Drop the regression and no-cicd tags from the new docker-loki eval (test 259); keep logs/easy/loki/fast - Remove the analysis/ directory used during the investigation Sweep numbers (n=5, opus-4.6, docker-loki eval, 259): metric baseline current this PR recovery time 78.6s 89.4s 83.0s 0.59 turns 7.8 10.6 8.6 0.71 total tokens 194K 259K 203K 0.86 cached 156K 220K 166K 0.85 cost $0.41 $0.45 $0.40 fully restored fetch_skill (Σ5) 0 29 0 — Signed-off-by: Claude <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (3)
analysis/2026-05-15-regression/the-fix.md (1)
71-92: ⚡ Quick winAdd language identifier to fenced code block.
The code block should specify
diffas the language identifier for proper syntax highlighting.📝 Proposed fix
-``` +```diff # 1. Add back the deleted section to generic_ask.jinja2 (between the🤖 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 `@analysis/2026-05-15-regression/the-fix.md` around lines 71 - 92, The fenced code block in the PR description is missing the language identifier for proper syntax highlighting; update the block around the changes to include the "diff" language tag so the added section for generic_ask.jinja2 and the removal instruction for holmes/plugins/skills/builtin/kubernetes-troubleshooting render correctly (refer to the added comments mentioning kubectl commands, kubectl_describe and the file generic_ask.jinja2 as well as the remove command for the kubernetes-troubleshooting skill).analysis/2026-05-15-regression/fix_a_k8s_section_iter_1.md (1)
15-17: ⚡ Quick winPin baseline reference to a commit SHA (or dated run ID).
Line 15’s “latest ci-benchmark experiment on master” is mutable over time. Use a concrete SHA/date so this regression artifact stays reproducible.
🤖 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 `@analysis/2026-05-15-regression/fix_a_k8s_section_iter_1.md` around lines 15 - 17, Replace the mutable text "Baseline: latest ci-benchmark experiment on master" with a concrete, pinned reference (e.g., a commit SHA or a dated run ID) so the regression artifact is reproducible; locate the exact line containing the phrase "Baseline: latest ci-benchmark experiment on master" in the file and update it to include the chosen immutable identifier (for example "Baseline: <commit-SHA> (run date: YYYY-MM-DD)" or "Baseline: <dated-run-ID>") and, if appropriate, add a brief parenthetical explaining the chosen identifier.analysis/2026-05-15-regression/fix-comparison.md (1)
3-3: 💤 Low valueConsider using recommended model naming format for future regression tests.
The model reference "opus-4.6 via OpenRouter" documents the actual test that was run. For future regression tests, consider using the recommended naming format (e.g.,
anthropic/claude-opus-4-5-20251101) for consistency and clarity. As per coding guidelines, documentation examples should reference models using the provider-prefixed format with date suffixes.🤖 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 `@analysis/2026-05-15-regression/fix-comparison.md` at line 3, The test documentation uses an informal model reference "opus-4.6 via OpenRouter"; update the test name/description to use the provider-prefixed, dated model naming convention (e.g., replace "opus-4.6 via OpenRouter" with a provider-prefixed form like "anthropic/claude-opus-4-6-<YYYYMMDD>" or the exact published tag), ensuring the string in the test identifier `259_loki_historical_logs_pod_deleted_docker` and any accompanying description uses the canonical format for consistency with other regression tests and guidelines.
🤖 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 `@analysis/2026-05-15-regression/n5-sweep.md`:
- Around line 12-24: The markdown tables under the "Baseline (`cf6ddb7`)" and
"Current (`1b61fe3`)" headings need blank lines inserted before and after each
table to satisfy markdownlint MD058; update the n5-sweep.md content by ensuring
there's an empty line between the headings and the table start, and another
empty line after the table end (i.e., add a blank line before the |--- row and
after the last | row for both tables).
In `@analysis/2026-05-15-regression/run_sweep.sh`:
- Around line 22-33: The script run_sweep.sh currently silences pytest failures
by appending "|| true" to the timeout/poetry pytest command, causing failed
iterations to be treated as success; remove "|| true" and instead capture the
pytest exit code from the timeout/poetry invocation (e.g., store "$?"/assign to
a variable immediately after the timeout poetry run pytest line) and propagate
failures by exiting non-zero (exit with that code) or set a failure flag and
exit non-zero at the end; ensure the test invocation and subsequent
evals_report.md handling still run when desired but that any pytest non-zero
exit causes the script to fail (reference the timeout 300 poetry run pytest ...
command and the surrounding conditional that checks evals_report.md).
In
`@tests/llm/fixtures/test_ask_holmes/259_loki_historical_logs_pod_deleted_docker/generate_logs.py`:
- Around line 20-22: Replace the hardcoded shared values by generating
test-specific resource names for fixture 259: update the constants NAMESPACE,
POD_NAME and SERVICE in generate_logs.py to use the test id (e.g. "app-259" and
"payment-api-259-<unique-suffix>") instead of "app-101"/"payment-api-101-*",
ensuring POD_NAME includes a unique per-test suffix; then update the
corresponding namespace and resource name references inside test_case.yaml
(queries/prompts) to match the new app-259 and payment-api-259 names so the
fixture and its test_case remain consistent and isolated.
In
`@tests/llm/fixtures/test_ask_holmes/259_loki_historical_logs_pod_deleted_docker/test_case.yaml`:
- Around line 1-16: The test oracle is too vague and can pass without querying
Loki; update the YAML by making expected_output assert a discoverable log detail
(e.g., the exact incident message substring and a tight time window like
"2025-08-02T13:45:00Z" or the log line "error: connection pool exhausted") so
only a real Loki response satisfies it, and enable tool-call verification by
adding include_tool_calls: true so the test harness can verify the grafana/loki
tool was invoked; adjust user_prompt only if needed to reference the exact
timestamp string used in expected_output.
---
Nitpick comments:
In `@analysis/2026-05-15-regression/fix_a_k8s_section_iter_1.md`:
- Around line 15-17: Replace the mutable text "Baseline: latest ci-benchmark
experiment on master" with a concrete, pinned reference (e.g., a commit SHA or a
dated run ID) so the regression artifact is reproducible; locate the exact line
containing the phrase "Baseline: latest ci-benchmark experiment on master" in
the file and update it to include the chosen immutable identifier (for example
"Baseline: <commit-SHA> (run date: YYYY-MM-DD)" or "Baseline: <dated-run-ID>")
and, if appropriate, add a brief parenthetical explaining the chosen identifier.
In `@analysis/2026-05-15-regression/fix-comparison.md`:
- Line 3: The test documentation uses an informal model reference "opus-4.6 via
OpenRouter"; update the test name/description to use the provider-prefixed,
dated model naming convention (e.g., replace "opus-4.6 via OpenRouter" with a
provider-prefixed form like "anthropic/claude-opus-4-6-<YYYYMMDD>" or the exact
published tag), ensuring the string in the test identifier
`259_loki_historical_logs_pod_deleted_docker` and any accompanying description
uses the canonical format for consistency with other regression tests and
guidelines.
In `@analysis/2026-05-15-regression/the-fix.md`:
- Around line 71-92: The fenced code block in the PR description is missing the
language identifier for proper syntax highlighting; update the block around the
changes to include the "diff" language tag so the added section for
generic_ask.jinja2 and the removal instruction for
holmes/plugins/skills/builtin/kubernetes-troubleshooting render correctly (refer
to the added comments mentioning kubectl commands, kubectl_describe and the file
generic_ask.jinja2 as well as the remove command for the
kubernetes-troubleshooting skill).
🪄 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: a6ec4951-a11a-4933-a20f-e189ca95646e
📒 Files selected for processing (50)
analysis/2026-05-15-regression/baseline_iter_1.mdanalysis/2026-05-15-regression/baseline_iter_2.mdanalysis/2026-05-15-regression/baseline_iter_3.mdanalysis/2026-05-15-regression/baseline_iter_4.mdanalysis/2026-05-15-regression/baseline_iter_5.mdanalysis/2026-05-15-regression/baseline_system_prompt.txtanalysis/2026-05-15-regression/baseline_user_prompt.txtanalysis/2026-05-15-regression/current_iter_1.mdanalysis/2026-05-15-regression/current_iter_2.mdanalysis/2026-05-15-regression/current_iter_3.mdanalysis/2026-05-15-regression/current_iter_4.mdanalysis/2026-05-15-regression/current_iter_5.mdanalysis/2026-05-15-regression/current_system_prompt.txtanalysis/2026-05-15-regression/current_user_prompt.txtanalysis/2026-05-15-regression/fix-comparison.mdanalysis/2026-05-15-regression/fix_a_k8s_section_iter_1.mdanalysis/2026-05-15-regression/fix_a_k8s_section_iter_2.mdanalysis/2026-05-15-regression/fix_a_k8s_section_iter_3.mdanalysis/2026-05-15-regression/fix_a_k8s_section_iter_4.mdanalysis/2026-05-15-regression/fix_a_k8s_section_iter_5.mdanalysis/2026-05-15-regression/fix_ab_iter_1.mdanalysis/2026-05-15-regression/fix_ab_iter_2.mdanalysis/2026-05-15-regression/fix_ab_iter_3.mdanalysis/2026-05-15-regression/fix_ab_iter_4.mdanalysis/2026-05-15-regression/fix_ab_iter_5.mdanalysis/2026-05-15-regression/fix_ac_iter_1.mdanalysis/2026-05-15-regression/fix_ac_iter_2.mdanalysis/2026-05-15-regression/fix_ac_iter_3.mdanalysis/2026-05-15-regression/fix_ac_iter_4.mdanalysis/2026-05-15-regression/fix_ac_iter_5.mdanalysis/2026-05-15-regression/fix_ad_iter_1.mdanalysis/2026-05-15-regression/fix_ad_iter_2.mdanalysis/2026-05-15-regression/fix_ad_iter_3.mdanalysis/2026-05-15-regression/fix_ad_iter_4.mdanalysis/2026-05-15-regression/fix_ad_iter_5.mdanalysis/2026-05-15-regression/fix_b_must_use_tools_iter_1.mdanalysis/2026-05-15-regression/fix_b_must_use_tools_iter_2.mdanalysis/2026-05-15-regression/fix_b_must_use_tools_iter_3.mdanalysis/2026-05-15-regression/fix_b_must_use_tools_iter_4.mdanalysis/2026-05-15-regression/fix_b_must_use_tools_iter_5.mdanalysis/2026-05-15-regression/n5-sweep.mdanalysis/2026-05-15-regression/run_sweep.shanalysis/2026-05-15-regression/system_prompt.diffanalysis/2026-05-15-regression/the-fix.mdanalysis/2026-05-15-regression/trace-diff.mdtests/llm/fixtures/test_ask_holmes/259_loki_historical_logs_pod_deleted_docker/generate_logs.pytests/llm/fixtures/test_ask_holmes/259_loki_historical_logs_pod_deleted_docker/loki-config.yamltests/llm/fixtures/test_ask_holmes/259_loki_historical_logs_pod_deleted_docker/test_case.yamltests/llm/fixtures/test_ask_holmes/259_loki_historical_logs_pod_deleted_docker/toolsets.yamltests/llm/utils/classifiers.py
- Rename namespace/pod to app-259 / payment-api-259-* (per CLAUDE.md: "Use dedicated namespace per test: app-<testid>") - Tighten expected_output to require a discoverable log detail (time window + exact ERROR substring) instead of a generic conclusion that the model could plausibly guess - Add include_tool_calls: true so the grader also verifies grafana/loki was actually called Verified locally with the docker-loki Loki container: 1/1 pass, 86.9s, 9 turns, 20 tool calls, $0.40 (opus-4.6 via OpenRouter). Leaving the Benchmark Master / Benchmark PR CI failures alone — both master and PR runs failed at the same point (~100s into the job), which means the failure is upstream of this PR (likely OpenRouter credits exhaustion at the time of run, same env issue we hit earlier). Signed-off-by: Claude <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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_loki_historical_logs_pod_deleted_docker/generate_logs.py`:
- Around line 26-27: Add Python type hints to the four functions missing
annotations: push, log_entry, generate, and add. For each function signature
(the def lines for push, log_entry, generate, and add) add explicit parameter
and return types consistent with the repository conventions (e.g., str for
loki_url, typing.Dict/typing.List for streams_by_level or entries, specific
types for timestamps/levels, and appropriate return types like None or
Iterable[...]); ensure you import any typing symbols used (e.g., List, Dict,
Iterable, Optional) at the top of the file and update all four function
definitions accordingly.
- Around line 71-75: Update all function signatures to include full type hints:
add parameter and return type annotations for push(), log_entry(), and
generate(), and add a return type for the nested add() function; reference the
functions named push, log_entry, generate, and add in the file so reviewers can
find them. Make datetime objects UTC-aware by constructing them with
tzinfo=timezone.utc (or using datetime(..., tz=timezone.utc)) for problem_start,
problem_end, current, and scenario_end so subsequent .timestamp() calls are
consistent across timezones. Ensure any imports required for typing (e.g., List,
Dict, Optional) and timezone (from datetime import timezone) are added or
adjusted accordingly. Verify that type hints match the actual returned values
(e.g., Dict[str, Any] or List[str]) used in push/log_entry/generate/add.
🪄 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: 01c3090b-1db4-4acd-9d1f-e1c0f22e1220
📒 Files selected for processing (2)
tests/llm/fixtures/test_ask_holmes/259_loki_historical_logs_pod_deleted_docker/generate_logs.pytests/llm/fixtures/test_ask_holmes/259_loki_historical_logs_pod_deleted_docker/test_case.yaml
Codifies four rules learned the hard way while investigating the PR #2040 eval regression: 1. Pull raw trace data BEFORE theorising from file diffs (file diffs misled this investigation twice — claimed prompt grew when it actually shrank in rendered form). 2. Compare per-iter tool-call sequences, not aggregate means (means hid the +1 deterministic fetch_skill turn under noise). 3. When bisecting a PR, read the FULL diff with git show --stat; half-reverts of just the file you noticed waste sweeps because the other side of the PR (registered skill, default config, etc.) still triggers the new behavior. 4. Deterministic 5/5 behavior needs trigger removal, not prompt softening — competing nudges rarely override a strong trigger like a registered skill in the catalog. Includes the ordered diagnostic sequence (traces first, then sweeps). Signed-off-by: Claude <noreply@anthropic.com>
Earlier version overgeneralized from a single investigation: the "trigger removal vs prompt softening" rule and the fixed 5-step diagnostic recipe both baked in the specific shape of the PR-#2040 regression (more LLM calls, deterministic skill fetch). They don't generalize to other regression shapes. Reduce to the actually-general principle: pull trace data before designing a fix; look at individual runs not just aggregates; and read the full diff of a suspect PR before reverting parts of it. Signed-off-by: Claude <noreply@anthropic.com>
…evals-regression-EYQQC
1. Prefer test_id over eval_id when building baseline rows
(tests/llm/utils/braintrust_history.py)
For parameterized tests like
'227_count_configmaps_per_namespace[0][opus-4.6][default]', the eval
metadata stores:
- eval_id = '227_count_configmaps_per_namespace' (strips [N])
- test_id = '227_count_configmaps_per_namespace[0]' (keeps it)
The report joins on test_case_name which always carries the
parameterization, so eval_id-keyed baseline rows silently fail to
match and the "vs master" / "vs benchmark" cells come back empty for
every parameterized eval. Falling back to eval_id keeps older spans
that only set the latter from disappearing.
Verified end-to-end against the live master-25943210368 experiment:
11/11 regression tests now match (was 10/11).
2. Pass GITHUB_TOKEN to the pytest step in eval-regression.yaml
tests/llm/utils/braintrust_history.py uses
_find_recent_workflow_run_ids() to map github action run IDs to
Braintrust experiment names (master-{run_id}, ci-benchmark-{run_id}).
Without GITHUB_TOKEN that call hits the 60 req/h unauthenticated
rate limit shared across GitHub Actions runner IPs, returns no run
IDs, and the master/benchmark baselines come back empty for the
entire report. The function reads the token from the env when
present (see line 174), it just wasn't being passed in.
Signed-off-by: Claude <noreply@anthropic.com>
The existing single summary row (now relabeled "Comparable") averaged each baseline column over its own matched subset and produced apples-to-apples deltas, but the "This branch" cell silently switched between two different test subsets depending on which baseline was available — which made the absolute number hard to read. Now each metric table emits two summary rows: - **Total (all, n=N)** — each column averaged over its own non-null subset across all rows. The "This branch" cell is the mean of every current value in the run, regardless of whether a baseline exists. Δ cells are intentionally empty because the per-column subsets may differ (e.g. baseline missing one test would make the delta compare different sets of tests — apples to oranges). - **Comparable (m=N, b=N)** — per-baseline matched subsets only. The vs-master pair is averaged across rows where both this-branch AND master have a value; same for vs-benchmark. Use this row for meaningful deltas; use the Total row for absolutes. Example (from PR 2051 where master had no data and benchmark was missing two tests): | Total (all, n=11) | 31.9s | — | — | 30.8s | — | | Comparable (m=0,b=9)| 36.3s | — | — | 30.8s | ↑18% | The 31.9s vs 36.3s gap is informative: the two tests missing from the benchmark happen to be short ones, so the comparable subset runs slower on average than the full run. Signed-off-by: Claude <noreply@anthropic.com>
aantn
changed the title
aantn
enabled auto-merge (squash)
May 16, 2026 17:32
moshemorad
approved these changes
May 17, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.