Skip to content

Clarify output_type parameter to prevent double conversion - #1798

Closed
aantn wants to merge 1 commit into
masterfrom
claude/apply-prometheus-diff-xxho4
Closed

aantn wants to merge 1 commit into
masterfrom
claude/apply-prometheus-diff-xxho4

Conversation

@aantn

@aantn aantn commented Mar 16, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

Updated the documentation for the output_type parameter in the Prometheus toolset to clarify that users should not manually convert values in their queries when using output type formatting options.

Key Changes

  • Enhanced the output_type parameter description to explicitly warn against manual conversions in queries
  • Added a concrete example showing how double conversion can occur (e.g., using * 100 in a query when output_type=Percentage is set)
  • Clarified that formatting conversions happen automatically at formatting time and should not be duplicated in the query itself

Details

The change addresses a common user error where manual value conversions in Prometheus queries would conflict with the automatic conversions performed by the output type formatter. This could result in incorrect values being returned to users. The updated documentation now makes it clear that users should provide raw metric values and let the output_type parameter handle all necessary transformations.

https://claude.ai/code/session_01Hc36WEwXrkTHFv9AJju431

Summary by CodeRabbit

Documentation

  • Improved parameter guidance to clarify correct configuration and prevent potential double conversion errors when using certain scaling operations.

…ersion

Adds explicit guidance not to manually convert values in the query when
using output_type formatting, as this causes double conversion.

https://claude.ai/code/session_01Hc36WEwXrkTHFv9AJju431
Signed-off-by: Claude <noreply@anthropic.com>
@claude

claude Bot commented Mar 16, 2026

Copy link
Copy Markdown
Contributor

Claude Code Review

This repository is configured for manual code reviews. Comment @claude review to trigger a review.

@github-actions

github-actions Bot commented Mar 16, 2026 •

Copy link
Copy Markdown
Contributor

✅ Results of HolmesGPT evals

Automatically triggered by commit f01fd02 on branch claude/apply-prometheus-diff-xxho4

View workflow logs

Results of HolmesGPT evals

  • ask_holmes: 10/10 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
✅ 09_crashpod 27.1s 5 11 $0.3459 105,806 103,657 23,679 2,149 884 61,537 42,120 — —
✅ 101_loki_historical_logs_pod_deleted 37.9s 6 12 $0.4187 134,006 131,218 25,028 2,788 845 81,754 49,464 — —
✅ 111_pod_names_contain_service 29.3s 5 11 $0.3337 103,169 101,051 22,821 2,118 586 60,940 40,111 — —
✅ 112_find_pvcs_by_uuid 13.9s 3 3 $0.2928 59,923 59,074 21,326 849 450 16,980 42,094 — —
✅ 12_job_crashing 34.3s 7 15 $0.4314 161,899 159,442 25,644 2,457 594 108,834 50,608 — —
✅ 176_network_policy_blocking_traffic_no_runbooks 40.5s 7 17 $0.5683 162,578 159,288 28,072 3,290 987 88,222 71,066 — —
✅ 227_count_configmaps_per_namespace[0] 19.7s 5 9 $0.3049 95,098 93,829 20,940 1,269 587 54,343 39,486 — —
✅ 24_misconfigured_pvc 30.6s 5 14 $0.3744 106,122 103,702 23,660 2,420 887 57,699 46,003 — —
✅ 43_current_datetime_from_prompt 3.7s 1 — $0.1091 17,072 16,945 16,945 127 127 0 16,945 — —
✅ 61_exact_match_counting 10.5s 3 3 $0.2426 53,090 52,650 17,994 440 293 16,944 35,706 — —
Total 24.8s avg 4.7 avg 10.6 avg $3.4217 998,763 980,856 28,072 17,907 987 547,253 433,603 — —
Benchmark Comparison Details

Baseline: latest ci-benchmark experiment on master

Status: Success - 73 test/model combinations loaded

Benchmark experiment:

No benchmark data available for comparison.

Benchmark has no cost, total tokens, cached tokens data. Will appear after the next weekly benchmark run.

Comparison indicators:

  • ±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/apply-prometheus-diff-xxho4 -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
filter: 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!)
filter 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 in automatic regression runs:

Label Effect
evals-tag-<name> Run tests with tag <name> alongside regression
evals-id-<name> Run a specific eval by test ID
evals-model-<name> Override the model (use model list name, e.g. sonnet-4.5)

Examples: evals-tag-easy, evals-id-09_crashpod, evals-model-sonnet-4.5

🏷️ Valid tags

benchmark, chain-of-causation, compaction, confluence, context_window, coralogix, counting, database, datadog, datetime, db-connectors, easy, elasticsearch, embeds, fast, frontend, grafana, hard, integration, kafka, kubernetes, leaked-information, logs, loki, medium, metrics, network, newrelic, no-cicd, numerical, one-test, port-forward, prometheus, question-answer, regression, runbooks, slackbot, storage, toolset-limitation, traces, transparency

🤖 Valid models

deepseek-chat, deepseek-r1-reasoner, deepseek-reasoner, deepseek-v3.2-chat, gemini-3-flash-preview, gemini-3-pro-preview, gemini-3.1-pro-preview, gpt-4.1, gpt-5.2-high-reasoning, gpt-5.3-codex, gpt-5.4, haiku-4.5, kimi-2.5, kimi-2.5-openrouter, opus-4.5, opus-4.6, qwen-next-80B-instruct, qwen-next-80B-thinking, sonnet-4.5, sonnet-4.6


Commands: /eval · /rerun · /list

CLI: gh workflow run eval-regression.yaml --repo HolmesGPT/holmesgpt --ref claude/apply-prometheus-diff-xxho4 -f markers=regression -f filter=

@github-actions

github-actions Bot commented Mar 16, 2026 •

Copy link
Copy Markdown
Contributor

✅ Docker images ready for 37ca3781 (built in 7m 33s)

⚠️ 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:37ca3781
docker tag us-central1-docker.pkg.dev/robusta-development/temporary-builds/holmes:37ca3781 me-west1-docker.pkg.dev/robusta-development/development/holmes-dev:37ca3781
docker push me-west1-docker.pkg.dev/robusta-development/development/holmes-dev:37ca3781
docker pull us-central1-docker.pkg.dev/robusta-development/temporary-builds/holmes-operator:37ca3781
docker tag us-central1-docker.pkg.dev/robusta-development/temporary-builds/holmes-operator:37ca3781 me-west1-docker.pkg.dev/robusta-development/development/holmes-operator-dev:37ca3781
docker push me-west1-docker.pkg.dev/robusta-development/development/holmes-operator-dev:37ca3781

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

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

@coderabbitai

coderabbitai Bot commented Mar 16, 2026 •

Copy link
Copy Markdown
Contributor

Walkthrough

Updated the parameter description for output_type in ExecuteRangeQuery to clarify proper usage and warn against double value conversion when using output_type=Percentage with in-query scaling.

Changes

Cohort / File(s) Summary
Prometheus ExecuteRangeQuery Documentation
holmes/plugins/toolsets/prometheus/prometheus.py
Added warning to output_type parameter description about potential double conversion with Percentage output type and in-query scaling.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Possibly related PRs

Suggested reviewers

  • moshemorad
  • arikalon1
🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: clarifying documentation for the output_type parameter to prevent double conversion in Prometheus queries.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

📝 Coding Plan
  • Generate coding plan for human review comments

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.

@netlify

netlify Bot commented Mar 16, 2026

Copy link
Copy Markdown

✅ Deploy Preview for holmes-docs ready!

Name Link
🔨 Latest commit f01fd02
🔍 Latest deploy log https://app.netlify.com/projects/holmes-docs/deploys/69b88e1f7689770008f7c4fe
😎 Deploy Preview https://deploy-preview-1798--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.

@github-actions

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 12.79s 11.67s +9.6%
Warm Mean 5.42s 5.22s +3.8%
Warm Min 5.36s 5.19s
Warm Max 5.49s 5.24s

🟡 Full CLI with LLM

Measures holmes ask execution time (OpenRouter + Haiku 4.5)

Metric PR Master Change
Cold Start 33.24s 26.00s +27.8%
Warm Mean 8.61s 8.72s -1.2%
Warm Min 8.37s 8.52s
Warm Max 8.84s 9.10s

PR: 37ca3781 | Master: 0b4d0434 | 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.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@holmes/plugins/toolsets/prometheus/prometheus.py`:
- Around line 1563-1567: The docstring for the ToolParameter named "output_type"
in prometheus.py contains EN DASH characters; replace any EN DASH (–) with a
standard hyphen-minus (-) in that string literal to satisfy Ruff and Python
conventions, locating the string assigned to "output_type" (the ToolParameter
call) and editing the description value to use simple ASCII hyphens where
needed.
🪄 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: 04ff8310-fc38-414d-9e95-17f40f7df046

📥 Commits

Reviewing files that changed from the base of the PR and between 0b4d043 and f01fd02.

📒 Files selected for processing (1)
  • holmes/plugins/toolsets/prometheus/prometheus.py

Comment on lines 1563 to 1567
"output_type": ToolParameter(
description="Specifies how to interpret the Prometheus result. Use 'Plain' for raw values, 'Bytes' to format byte values, 'Percentage' to scale 0–1 values into 0–100%, or 'CPUUsage' to convert values to cores (e.g., 500 becomes 500m, 2000 becomes 2).",
description="Specifies how to interpret the Prometheus result. Use 'Plain' for raw values, 'Bytes' to format byte values, 'Percentage' to scale 0–1 values into 0–100%, or 'CPUUsage' to convert values to cores (e.g., 500 becomes 500m, 2000 becomes 2). Do NOT convert on your own in the query. E.g. if setting output_type=Percentage it is a mistake to have * 100 in the query and convert numbers yourself in the query from 0-1 range to 0-100. This will cause a double conversion as further conversion happens at formatting time",
type="string",
required=True,
),

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.

⚠️ Potential issue | 🟡 Minor

Replace EN DASH with HYPHEN-MINUS for consistency.

The documentation improvement clearly explains the output_type behavior and warns against double conversion—well done. However, Ruff flags ambiguous EN DASH characters (–) that should be HYPHEN-MINUS (-) per Python string conventions.

🔧 Proposed fix
                 "output_type": ToolParameter(
-                    description="Specifies how to interpret the Prometheus result. Use 'Plain' for raw values, 'Bytes' to format byte values, 'Percentage' to scale 0–1 values into 0–100%, or 'CPUUsage' to convert values to cores (e.g., 500 becomes 500m, 2000 becomes 2). Do NOT convert on your own in the query. E.g. if setting output_type=Percentage it is a mistake to have * 100 in the query and convert numbers yourself in the query from 0-1 range to 0-100. This will cause a double conversion as further conversion happens at formatting time",
+                    description="Specifies how to interpret the Prometheus result. Use 'Plain' for raw values, 'Bytes' to format byte values, 'Percentage' to scale 0-1 values into 0-100%, or 'CPUUsage' to convert values to cores (e.g., 500 becomes 500m, 2000 becomes 2). Do NOT convert on your own in the query. E.g. if setting output_type=Percentage it is a mistake to have * 100 in the query and convert numbers yourself in the query from 0-1 range to 0-100. This will cause a double conversion as further conversion happens at formatting time",
                     type="string",
                     required=True,
                 ),
🧰 Tools
🪛 Ruff (0.15.6)

[warning] 1564-1564: String contains ambiguous – (EN DASH). Did you mean - (HYPHEN-MINUS)?

(RUF001)


[warning] 1564-1564: String contains ambiguous – (EN DASH). Did you mean - (HYPHEN-MINUS)?

(RUF001)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@holmes/plugins/toolsets/prometheus/prometheus.py` around lines 1563 - 1567,
The docstring for the ToolParameter named "output_type" in prometheus.py
contains EN DASH characters; replace any EN DASH (–) with a standard
hyphen-minus (-) in that string literal to satisfy Ruff and Python conventions,
locating the string assigned to "output_type" (the ToolParameter call) and
editing the description value to use simple ASCII hyphens where needed.

@mershal mershal closed this Mar 19, 2026
@mershal
mershal deleted the claude/apply-prometheus-diff-xxho4 branch March 19, 2026 14:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants