[ROB-3883] Loki returns raw response on parse failure - #2035
Merged
Merged
Conversation
Signed-off-by: avi@robusta.dev <avi@robusta.dev>
When Loki returns a malformed JSON body, the toolset previously surfaced only the parse error message (e.g. "Expecting ',' delimiter: line 1 column 2811"), giving no insight into what was actually returned. This is particularly painful when a proxy (e.g. nginx with body_filter_by_lua scrubbing) is corrupting the response in transit — the error position is on the corrupted body, not the original. Catch the ValueError from response.json() and re-raise with the raw response text, length, and content-type included so the LLM (and operator) can see the actual bytes that came back. Signed-off-by: avi@robusta.dev <avi@robusta.dev>
The previous commit only included the raw response when JSON parsing failed. Widen the net to any exception raised after the HTTP response is in hand (e.g. unexpected response shape in parse_loki_response, KeyError on missing fields), so the raw bytes are always available for debugging — not just for JSONDecodeError. The split into two try/except blocks keeps the RequestException path (no response object yet) separate from the response-handling path (response.text is available). Signed-off-by: avi@robusta.dev <avi@robusta.dev>
This reverts commit ef07978. Signed-off-by: avi@robusta.dev <avi@robusta.dev>
Signed-off-by: avi@robusta.dev <avi@robusta.dev>
![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📜 #3 · Run @ __fbd82a1__ (#25988798786) — May 17, 10:59 UTC✅ Results of HolmesGPT evalsAutomatically triggered by commit fbd82a1 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 | 35.1s | 5 | 10 | $0.2675 | 104,901 | 102,822 | 23,604 | 2,079 | 652 | 78,313 | 24,509 | 196 | — | src |
| ✅ | 101_loki_historical_logs_pod_deleted | 76.4s | 7 | 18 | $0.4440 | 178,284 | 173,245 | 31,057 | 5,039 | 922 | 139,415 | 33,830 | 925 | — | src |
| ✅ | 112_find_pvcs_by_uuid | 21.3s | 3 | 5 | $0.2141 | 61,904 | 60,533 | 22,233 | 1,371 | 777 | 37,800 | 22,733 | 264 | — | src |
| ✅ | 12_job_crashing | 38.3s | 6 | 12 | $0.2989 | 131,831 | 129,597 | 24,133 | 2,234 | 776 | 102,720 | 26,877 | 170 | — | src |
| ✅ | 176_network_policy_blocking_traffic_no_skills | 54.5s | 7 | 17 | $0.3753 | 166,953 | 163,312 | 27,902 | 3,641 | 842 | 133,614 | 29,698 | 667 | — | src |
| ✅ | 227_count_configmaps_per_namespace[0] | 19.0s | 4 | 9 | $0.2085 | 76,280 | 75,153 | 20,551 | 1,127 | 593 | 53,356 | 21,797 | 53 | — | src |
| ✅ | 243_pod_names_contain_service | 30.0s | 4 | 8 | $0.2330 | 78,925 | 77,083 | 21,683 | 1,842 | 872 | 54,833 | 22,250 | 265 | — | src |
| ✅ | 24_misconfigured_pvc | 38.3s | 5 | 14 | $0.2862 | 106,562 | 103,984 | 23,608 | 2,578 | 1,057 | 78,591 | 25,393 | 222 | — | src |
| ✅ | 43_current_datetime_from_prompt | 3.9s | 1 | — | $0.1182 | 16,898 | 16,798 | 16,798 | 100 | 100 | 0 | 16,798 | 60 | — | src |
| ✅ | 51_logs_summarize_errors | 21.2s | 4 | 5 | $0.2044 | 76,671 | 75,523 | 20,757 | 1,148 | 368 | 54,761 | 20,762 | 84 | — | src |
| ✅ | 61_exact_match_counting | 10.3s | 3 | 3 | $0.1511 | 52,433 | 52,071 | 17,780 | 362 | 215 | 34,287 | 17,784 | 31 | — | src |
| Total | 31.7s avg | 4.5 avg | 10.1 avg | $2.8013 | 1,051,642 | 1,030,121 | 31,057 | 21,521 | 1,057 | 767,690 | 262,431 | 2,937 | — |
Benchmark Comparison Details
Master baseline: latest master-* experiment (post-merge regression eval)
Status: 11 test/model combinations loaded
- master-25988085058 (created: 2026-05-17)
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 (1h ago) | Δ vs master | benchmark (14d ago) | Δ vs benchmark |
|---|---|---|---|---|---|
| 09_crashpod (opus-4.6) 📄 | 35.1s | 37.8s | ±0% | 32.5s | ±0% |
| 101_loki_historical_logs_pod_deleted (opus-4.6) 📄 | 76.4s | 79.5s | ±0% | 51.7s | ↑48% |
| 112_find_pvcs_by_uuid (opus-4.6) 📄 | 21.3s | 20.7s | ±0% | 18.1s | ↑18% |
| 12_job_crashing (opus-4.6) 📄 | 38.3s | 41.9s | ±0% | 42.4s | ±0% |
| 176_network_policy_blocking_traffic_no_skills (opus-4.6) 📄 | 54.5s | 46.8s | ↑16% | 35.7s | ↑53% |
| 227_count_configmaps_per_namespace[0] (opus-4.6) 📄 | 19.0s | 19.9s | ±0% | 19.7s | ±0% |
| 243_pod_names_contain_service (opus-4.6) 📄 | 30.0s | 34.2s | ↓12% | 27.4s | ±0% |
| 24_misconfigured_pvc (opus-4.6) 📄 | 38.3s | 37.6s | ±0% | 35.8s | ±0% |
| 43_current_datetime_from_prompt (opus-4.6) 📄 | 3.9s | 3.4s | ↑16% | — | — |
| 51_logs_summarize_errors (opus-4.6) 📄 | 21.2s | 22.4s | ±0% | 23.1s | ±0% |
| 61_exact_match_counting (opus-4.6) 📄 | 10.3s | 10.3s | ±0% | 10.8s | ±0% |
| Total (all, n=11) | 31.7s | 32.2s | — | 29.7s | — |
| Comparable (m=11, b=10) | 31.7s | 32.2s | ±0% | 29.7s | ↑16% |
Cost comparison:
| Test case | This branch | master (1h ago) | Δ vs master | benchmark (14d ago) | Δ vs benchmark |
|---|---|---|---|---|---|
| 09_crashpod (opus-4.6) 📄 | $0.2675 | $0.2829 | ±0% | $0.2616 | ±0% |
| 101_loki_historical_logs_pod_deleted (opus-4.6) 📄 | $0.4440 | $0.4365 | ±0% | $0.3371 | ↑32% |
| 112_find_pvcs_by_uuid (opus-4.6) 📄 | $0.2141 | $0.2049 | ±0% | $0.2014 | ±0% |
| 12_job_crashing (opus-4.6) 📄 | $0.2989 | $0.2999 | ±0% | $0.3076 | ±0% |
| 176_network_policy_blocking_traffic_no_skills (opus-4.6) 📄 | $0.3753 | $0.3208 | ↑17% | $0.2914 | ↑29% |
| 227_count_configmaps_per_namespace[0] (opus-4.6) 📄 | $0.2085 | $0.2038 | ±0% | $0.2059 | ±0% |
| 243_pod_names_contain_service (opus-4.6) 📄 | $0.2330 | $0.2566 | ±0% | $0.2280 | ±0% |
| 24_misconfigured_pvc (opus-4.6) 📄 | $0.2862 | $0.2858 | ±0% | $0.2831 | ±0% |
| 43_current_datetime_from_prompt (opus-4.6) 📄 | $0.1182 | $0.1182 | ±0% | — | — |
| 51_logs_summarize_errors (opus-4.6) 📄 | $0.2044 | $0.2025 | ±0% | $0.2072 | ±0% |
| 61_exact_match_counting (opus-4.6) 📄 | $0.1511 | $0.1511 | ±0% | $0.1522 | ±0% |
| Total (all, n=11) | $0.2547 | $0.2512 | — | $0.2475 | — |
| Comparable (m=11, b=10) | $0.2547 | $0.2512 | ±0% | $0.2475 | ±0% |
Total tokens comparison:
| Test case | This branch | master (1h ago) | Δ vs master | benchmark (14d ago) | Δ vs benchmark |
|---|---|---|---|---|---|
| 09_crashpod (opus-4.6) 📄 | 104,901 | 108,034 | ±0% | 103,497 | ±0% |
| 101_loki_historical_logs_pod_deleted (opus-4.6) 📄 | 178,284 | 195,566 | ±0% | 138,670 | ↑29% |
| 112_find_pvcs_by_uuid (opus-4.6) 📄 | 61,904 | 60,781 | ±0% | 61,169 | ±0% |
| 12_job_crashing (opus-4.6) 📄 | 131,831 | 133,114 | ±0% | 133,893 | ±0% |
| 176_network_policy_blocking_traffic_no_skills (opus-4.6) 📄 | 166,953 | 114,683 | ↑46% | 111,145 | ↑50% |
| 227_count_configmaps_per_namespace[0] (opus-4.6) 📄 | 76,280 | 76,264 | ±0% | 76,945 | ±0% |
| 243_pod_names_contain_service (opus-4.6) 📄 | 78,925 | 101,645 | ↓22% | 79,525 | ±0% |
| 24_misconfigured_pvc (opus-4.6) 📄 | 106,562 | 106,626 | ±0% | 108,047 | ±0% |
| 43_current_datetime_from_prompt (opus-4.6) 📄 | 16,898 | 16,898 | ±0% | — | — |
| 51_logs_summarize_errors (opus-4.6) 📄 | 76,671 | 76,361 | ±0% | 77,707 | ±0% |
| 61_exact_match_counting (opus-4.6) 📄 | 52,433 | 52,424 | ±0% | 52,942 | ±0% |
| Total (all, n=11) | 95,604 | 94,763 | — | 94,354 | — |
| Comparable (m=11, b=10) | 95,604 | 94,763 | ±0% | 94,354 | ±0% |
Cached tokens comparison:
| Test case | This branch | master (1h ago) | Δ vs master | benchmark (14d ago) | Δ vs benchmark |
|---|---|---|---|---|---|
| 09_crashpod (opus-4.6) 📄 | 78,313 | 80,131 | ±0% | 77,391 | ±0% |
| 101_loki_historical_logs_pod_deleted (opus-4.6) 📄 | 139,415 | 159,018 | ↓12% | 106,565 | ↑31% |
| 112_find_pvcs_by_uuid (opus-4.6) 📄 | 37,800 | 37,590 | ±0% | 38,002 | ±0% |
| 12_job_crashing (opus-4.6) 📄 | 102,720 | 105,176 | ±0% | 104,761 | ±0% |
| 176_network_policy_blocking_traffic_no_skills (opus-4.6) 📄 | 133,614 | 84,084 | ↑59% | 81,519 | ↑64% |
| 227_count_configmaps_per_namespace[0] (opus-4.6) 📄 | 53,356 | 54,279 | ±0% | 54,503 | ±0% |
| 243_pod_names_contain_service (opus-4.6) 📄 | 54,833 | 76,327 | ↓28% | 55,513 | ±0% |
| 24_misconfigured_pvc (opus-4.6) 📄 | 78,591 | 78,821 | ±0% | 80,270 | ±0% |
| 43_current_datetime_from_prompt (opus-4.6) 📄 | — | — | — | — | — |
| 51_logs_summarize_errors (opus-4.6) 📄 | 54,761 | 54,618 | ±0% | 55,443 | ±0% |
| 61_exact_match_counting (opus-4.6) 📄 | 34,287 | 34,283 | ±0% | 34,632 | ±0% |
| Total (all, n=11) | 69,790 | 69,484 | — | 68,860 | — |
| Comparable (m=10, b=10) | 76,769 | 76,433 | ±0% | 68,860 | ↑11% |
Turns comparison:
| Test case | This branch | master (1h ago) | Δ vs master | benchmark (14d ago) | Δ vs benchmark |
|---|---|---|---|---|---|
| 09_crashpod (opus-4.6) 📄 | 5 | 5 | ±0% | — | — |
| 101_loki_historical_logs_pod_deleted (opus-4.6) 📄 | 7 | 8 | ↓12% | — | — |
| 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) 📄 | 7 | 5 | ↑40% | — | — |
| 227_count_configmaps_per_namespace[0] (opus-4.6) 📄 | 4 | 4 | ±0% | — | — |
| 243_pod_names_contain_service (opus-4.6) 📄 | 4 | 5 | ↓20% | — | — |
| 24_misconfigured_pvc (opus-4.6) 📄 | 5 | 5 | ±0% | — | — |
| 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.5 | 4.5 | — | — | — |
| Comparable (m=11, b=0) | 4.5 | 4.5 | ±0% | — | — |
Tool calls comparison:
| Test case | This branch | master (1h ago) | Δ vs master | benchmark (14d ago) | Δ vs benchmark |
|---|---|---|---|---|---|
| 09_crashpod (opus-4.6) 📄 | 10 | 11 | ±0% | 10 | ±0% |
| 101_loki_historical_logs_pod_deleted (opus-4.6) 📄 | 18 | 18 | ±0% | 14 | ↑29% |
| 112_find_pvcs_by_uuid (opus-4.6) 📄 | 5 | 4 | ↑25% | 4 | ↑25% |
| 12_job_crashing (opus-4.6) 📄 | 12 | 13 | ±0% | 14 | ↓14% |
| 176_network_policy_blocking_traffic_no_skills (opus-4.6) 📄 | 17 | 13 | ↑31% | 13 | ↑31% |
| 227_count_configmaps_per_namespace[0] (opus-4.6) 📄 | 9 | 9 | ±0% | 9 | ±0% |
| 243_pod_names_contain_service (opus-4.6) 📄 | 8 | 9 | ↓11% | 8 | ±0% |
| 24_misconfigured_pvc (opus-4.6) 📄 | 14 | 15 | ±0% | 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.2 | 10.0 | — | 9.4 | — |
| Comparable (m=10, b=10) | 10.1 | 10.0 | ±0% | 9.4 | ±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:/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 0.26.0-loki-debug -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 0.26.0-loki-debug -f markers=regression -f filter=
Contributor
WalkthroughThis pull request refactors Loki query error handling to separate request failures (caught and re-raised immediately) from response JSON/processing failures (caught separately and raised with raw body, length, and Content-Type). Tests are updated to use per-test skipping and include a new malformed-JSON assertion. ChangesLoki Query Error Handling and Tests
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 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:a48eeca3
docker tag us-central1-docker.pkg.dev/robusta-development/temporary-builds/holmes:a48eeca3 me-west1-docker.pkg.dev/robusta-development/development/holmes-dev:a48eeca3
docker push me-west1-docker.pkg.dev/robusta-development/development/holmes-dev:a48eeca3
docker pull us-central1-docker.pkg.dev/robusta-development/temporary-builds/holmes-operator:a48eeca3
docker tag us-central1-docker.pkg.dev/robusta-development/temporary-builds/holmes-operator:a48eeca3 me-west1-docker.pkg.dev/robusta-development/development/holmes-operator-dev:a48eeca3
docker push me-west1-docker.pkg.dev/robusta-development/development/holmes-operator-dev:a48eeca3Patch 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:a48eeca3 \
--set operator.registry=me-west1-docker.pkg.dev/robusta-development/development \
--set operator.image=holmes-operator-dev:a48eeca3Robusta 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:a48eeca3 \
--set holmes.operator.registry=me-west1-docker.pkg.dev/robusta-development/development \
--set holmes.operator.image=holmes-operator-dev:a48eeca3 |
✅ Deploy Preview for holmes-docs ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Contributor
🔬 CLI Performance Benchmark🟡 Startup Time (no LLM)Measures
🟡 Full CLI with LLMMeasures
PR: |
![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: 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 `@holmes/plugins/toolsets/grafana/loki_api.py`:
- Around line 77-83: The exception currently embeds the full response.text (raw)
which can leak sensitive or unbounded data; modify the error construction in the
Loki response handling (where raw, response, and e are used) to include a
sanitized, bounded preview: define a MAX_PREVIEW (e.g. 1000) and build a preview
by truncating raw to MAX_PREVIEW and appending an explicit "(truncated)" marker,
strip or replace non-printable/control characters (or collapse long whitespace)
to avoid log injection, and report the preview length alongside
response.headers.get('Content-Type', 'unknown') and the original exception e
instead of inserting the entire raw payload. Ensure the final message still
gives context but never includes the full unbounded raw string.
- Around line 68-69: Replace the bare raise Exception in the requests exception
handler with raise RuntimeError(... ) from e to preserve the original traceback
(keep the existing message string but use RuntimeError and include "from e"),
and in the response processing block that calls response.json() and
parse_loki_response, narrow the except clauses to catch json.JSONDecodeError and
ValueError (for tuple unpacking) instead of a broad except, wrapping them in
RuntimeError(... ) from e; reference the existing
requests.exceptions.RequestException handler and the parse_loki_response call to
locate where to change the exception types and add exception chaining.
🪄 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: 6ff22c00-0c4a-43ae-b2d8-08ed2bfbbdea
📒 Files selected for processing (1)
holmes/plugins/toolsets/grafana/loki_api.py
moshemorad
previously approved these changes
May 17, 2026
Add a unit test that mocks the Loki query_range endpoint with a malformed JSON body and verifies the exception raised by execute_loki_query includes both the JSON parser error and the raw response text (with content-type and byte count), matching the debug behavior added earlier on this branch. Also scope the existing module-level "Grafana not running" skip to the integration tests via a @needs_grafana marker, so the new unit test is not skipped on machines without a local Grafana. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Signed-off-by: avi@robusta.dev <avi@robusta.dev>
moshemorad
approved these changes
May 17, 2026
Avi-Robusta
enabled auto-merge (squash)
May 17, 2026 11:02
Contributor
There was a problem hiding this comment.
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/plugins/toolsets/grafana/test_grafana.py`:
- Around line 14-18: The skip guard currently uses _grafana_skip_reason and
needs_grafana (from check_service_running("Grafana", 3000)) but
test_loki_toolset_direct_health_check calls Loki on port 3100; add a separate
_loki_skip_reason = check_service_running("Loki", 3100) and a needs_loki
pytest.mark.skipif using that reason (or replace the existing needs_grafana
marker on test_loki_toolset_direct_health_check with needs_loki) so the
Loki-specific test is skipped when Loki is unavailable; reference the symbols
_grafana_skip_reason, needs_grafana, _loki_skip_reason, needs_loki,
check_service_running, and test_loki_toolset_direct_health_check to locate and
update the code.
🪄 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: c13b141b-67ef-4201-8bc9-40da88c37b9c
📒 Files selected for processing (1)
tests/plugins/toolsets/grafana/test_grafana.py
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
2 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.
Summary by CodeRabbit
Bug Fixes
Tests