Repository navigation
Increase default max_points to 500 and allow LLM override up to 1000 - #1566
Conversation
The default MAX_GRAPH_POINTS was 100 with a hardcoded default of 60 data points, making graphs look completely different from Grafana. For a 6-hour range, Holmes showed 60 points (1 per 6 min) while Grafana shows ~1440 points at 15s resolution. Changes: - Raise MAX_GRAPH_POINTS default from 100 to 500 - Default step now targets max_points (not hardcoded 60) - Allow LLM to request higher resolution (up to 5x default) for simple single-series queries - Token-based truncation remains as safety net for large responses https://claude.ai/code/session_0174s8zNS3peCtHc2tg9iJ8y Signed-off-by: Claude <noreply@anthropic.com>
…flows For high-cardinality queries (many time series), a 5x override could generate responses that exceed the tool call token budget. Reducing to 2x still allows meaningful resolution increase for low-cardinality queries while keeping token usage reasonable. The tool description now explicitly guides the LLM to only increase above default for queries returning 1-3 series. https://claude.ai/code/session_0174s8zNS3peCtHc2tg9iJ8y Signed-off-by: Claude <noreply@anthropic.com>
The LLM had no guidance on how resolution affects data quality. Added instructions section explaining: - Default 500 points per series, controllable via max_points - Increase max_points (up to 1000) when investigating spikes/anomalies since they can disappear at lower resolution - Only increase for low-cardinality queries (1-3 series) - Narrow the time range for even higher resolution Also improved the step parameter description. https://claude.ai/code/session_0174s8zNS3peCtHc2tg9iJ8y Signed-off-by: Claude <noreply@anthropic.com>
https://claude.ai/code/session_0174s8zNS3peCtHc2tg9iJ8y Signed-off-by: Claude <noreply@anthropic.com>
✅ Deploy Preview for holmes-docs ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
📂 Previous Runs📜 Run @ 22cd0c0 (#22060892440)✅ Results of HolmesGPT evalsAutomatically triggered by commit 22cd0c0 on branch Results of HolmesGPT evals
📜 Run @ 4b5c604 (#22039013933)✅ Results of HolmesGPT evalsAutomatically triggered by commit 4b5c604 on branch Results of HolmesGPT evals
📜 Run @ 9f6de64 (#22038847415)✅ Results of HolmesGPT evalsAutomatically triggered by commit 9f6de64 on branch Results of HolmesGPT evals
📜 Run @ 49ccaa4 (#22038698023)✅ Results of HolmesGPT evalsAutomatically triggered by commit 49ccaa4 on branch Results of HolmesGPT evals
📜 Run @ 0080e46 (#22034151619)✅ Results of HolmesGPT evalsAutomatically triggered by commit 0080e46 on branch Results of HolmesGPT evals
✅ Results of HolmesGPT evalsAutomatically triggered by commit 13dfd9f on branch Results of HolmesGPT evals
📖 Legend
🔄 Re-run evals manually
Option 1: Comment on this PR with Or with more options (one per line): Run evals on a different branch (e.g., master) for comparison:
Quick re-run: Use Option 2: Trigger via GitHub Actions UI → "Run workflow" 🏷️ Valid markers
Commands: CLI: |
|
✅ Docker image ready for
Use this tag to pull the image for testing. 📋 Copy commandsgcloud auth configure-docker us-central1-docker.pkg.dev
docker pull us-central1-docker.pkg.dev/robusta-development/temporary-builds/holmes:dd1293b
docker tag us-central1-docker.pkg.dev/robusta-development/temporary-builds/holmes:dd1293b me-west1-docker.pkg.dev/robusta-development/development/holmes-dev:dd1293b
docker push me-west1-docker.pkg.dev/robusta-development/development/holmes-dev:dd1293bPatch 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:dd1293bRobusta 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:dd1293b |
|
No actionable comments were generated in the recent review. 🎉 WalkthroughDefault MAX_GRAPH_POINTS increased from 100 to 300 and a new MAX_GRAPH_POINTS_HARD_LIMIT was added; Prometheus step-calculation now supports a max_points override clamped by a hard limit and prompt rendering switched to load_and_render_prompt; prompts and tests updated accordingly. (49 words) Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 1 | ❌ 3❌ Failed checks (3 warnings)
✅ Passed checks (1 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 |
|
/eval |
🔬 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}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
holmes/plugins/toolsets/prometheus/prometheus.py (1)
481-498:⚠️ Potential issue | 🟡 MinorPotential
ZeroDivisionErrorifMAX_GRAPH_POINTSis set to 0.If someone sets the environment variable
MAX_GRAPH_POINTS=0, bothhard_limitandmax_pointswill be 0, causing aZeroDivisionErroron line 507 (time_range_seconds / max_points) and line 516. Consider adding a guard at the top of the function.🛡️ Proposed fix
hard_limit = MAX_GRAPH_POINTS * 2 + if hard_limit < 2: + hard_limit = 1000 # sensible fallback # Use override if provided and valid, otherwise use default - max_points = MAX_GRAPH_POINTS + max_points = MAX_GRAPH_POINTS if MAX_GRAPH_POINTS >= 1 else 500
🧹 Nitpick comments (3)
holmes/plugins/toolsets/prometheus/prometheus_instructions.jinja2 (1)
31-35: Hardcoded data point values (500, 1000) may drift fromMAX_GRAPH_POINTS.The instructions reference specific numbers (500 default, 1000 max) that are actually derived from the
MAX_GRAPH_POINTSenvironment variable. If an operator customizesMAX_GRAPH_POINTS, these instructions will be inaccurate. Consider templating these values if the variable is accessible in the template context.tests/plugins/toolsets/test_prometheus_unit.py (1)
56-59: Repeated local imports ofprom_modulecould be hoisted to module level.The
import holmes.plugins.toolsets.prometheus.prometheus as prom_moduleis duplicated in every test function/method. Moving it to the top of the file would comply with the coding guideline and reduce repetition. The monkeypatch will still work correctly since it patches the module's attribute.As per coding guidelines, "ALWAYS place Python imports at the top of the file, not inside functions or methods".
Also applies to: 103-105, 116-118, 131-133, 148-150, 163-165, 179-181
holmes/plugins/toolsets/prometheus/prometheus.py (1)
457-522: Twoadjust_step_for_max_pointsimplementations with different signatures exist and are intentionally used by different toolsets.The
utils.pyversion (time_range_seconds: int, max_points: int, step: Optional[int]) is used by Tempo (grafana), while the Prometheus version (start_timestamp: str, end_timestamp: str, step: Optional[float], max_points_override: Optional[float]) is used by Prometheus itself. The Prometheus implementation is more sophisticated, handling timestamp parsing and max_points override validation with hard limits and logging. This differentiation is appropriate for their respective toolsets, but worth noting for future maintenance if changes are made to either implementation.
|
@aantn Your eval run has finished. 🧪 Manual Eval Results
Results of HolmesGPT evals
|
| 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/fix-prometheus-downsampling-jZZnp -f markers=regression -f filter=
Option 1: Comment on this PR with /eval:
/eval
markers: regression
Or with more options (one per line):
/eval
model: gpt-4o
markers: regression
filter: 09_crashpod
iterations: 5
Run evals on a different branch (e.g., master) for comparison:
/eval
branch: master
markers: regression
| Option | Description |
|---|---|
model |
Model(s) to test (default: same as automatic runs) |
markers |
Pytest 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"
🏷️ Valid markers
benchmark, chain-of-causation, compaction, confluence, context_window, coralogix, counting, database, datadog, datetime, easy, elasticsearch, embeds, fast, frontend, grafana-dashboard, 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
Commands: /eval · /rerun · /list
CLI: gh workflow run eval-regression.yaml --repo HolmesGPT/holmesgpt --ref claude/fix-prometheus-downsampling-jZZnp -f markers=regression -f filter=
|
/eval |
|
@aantn Your eval run has finished. 🧪 Manual Eval Results
Results of HolmesGPT evals (branch:
|
| 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 master -f markers=regression -f filter=
Option 1: Comment on this PR with /eval:
/eval
markers: regression
Or with more options (one per line):
/eval
model: gpt-4o
markers: regression
filter: 09_crashpod
iterations: 5
Run evals on a different branch (e.g., master) for comparison:
/eval
branch: master
markers: regression
| Option | Description |
|---|---|
model |
Model(s) to test (default: same as automatic runs) |
markers |
Pytest 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"
🏷️ Valid markers
benchmark, chain-of-causation, compaction, confluence, context_window, coralogix, counting, database, datadog, datetime, easy, elasticsearch, embeds, fast, frontend, grafana-dashboard, 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
Commands: /eval · /rerun · /list
CLI: gh workflow run eval-regression.yaml --repo HolmesGPT/holmesgpt --ref master -f markers=regression -f filter=
… numbers
- Template now uses {{ default_max_points }} and {{ hard_max_points }} from
the MAX_GRAPH_POINTS env var so instructions stay accurate if the default changes
- Updated max_points tool description: removed "overview graphs" wording,
clarified decrease is to avoid hitting the data point limit
https://claude.ai/code/session_0174s8zNS3peCtHc2tg9iJ8y
Signed-off-by: Claude <noreply@anthropic.com>
….0.0.1:56587/git/HolmesGPT/holmesgpt into claude/fix-prometheus-downsampling-jZZnp
Co-locate with MAX_GRAPH_POINTS for better locality. Defaults to MAX_GRAPH_POINTS * 2 but can now be configured independently. https://claude.ai/code/session_0174s8zNS3peCtHc2tg9iJ8y Signed-off-by: Claude <noreply@anthropic.com>
|
/eval |
|
@aantn Your eval run has finished. 🧪 Manual Eval Results
Results of HolmesGPT evals
|
| 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/fix-prometheus-downsampling-jZZnp -f markers=regression -f filter=
Option 1: Comment on this PR with /eval:
/eval
markers: regression
Or with more options (one per line):
/eval
model: gpt-4o
markers: regression
filter: 09_crashpod
iterations: 5
Run evals on a different branch (e.g., master) for comparison:
/eval
branch: master
markers: regression
| Option | Description |
|---|---|
model |
Model(s) to test (default: same as automatic runs) |
markers |
Pytest 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"
🏷️ Valid markers
benchmark, chain-of-causation, compaction, confluence, context_window, coralogix, counting, database, datadog, datetime, easy, elasticsearch, embeds, fast, frontend, grafana-dashboard, 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
Commands: /eval · /rerun · /list
CLI: gh workflow run eval-regression.yaml --repo HolmesGPT/holmesgpt --ref claude/fix-prometheus-downsampling-jZZnp -f markers=regression -f filter=
Also fix tests to monkeypatch MAX_GRAPH_POINTS_HARD_LIMIT alongside MAX_GRAPH_POINTS so they remain independent of the default values. https://claude.ai/code/session_0174s8zNS3peCtHc2tg9iJ8y Signed-off-by: Claude <noreply@anthropic.com>
…1566) ## Summary This PR improves Prometheus query resolution by increasing the default maximum data points from 100 to 500, and allowing the LLM to request up to 1000 points (2x the default) for high-resolution analysis of low-cardinality queries. The change includes updated logic, comprehensive tests, and improved documentation. ## Key Changes - **Increased default MAX_GRAPH_POINTS**: Changed from 100 to 500 to provide better default resolution for time series queries - **Implemented hard limit for max_points override**: LLM can now request up to 2x MAX_GRAPH_POINTS (1000 points) for higher resolution, with a hard cap to prevent excessive data retrieval - **Updated step calculation logic**: When no step is provided, the default now targets the configured max_points instead of a fixed 60-point target - **Enhanced parameter descriptions**: Updated tool parameter documentation to clarify: - `step` parameter now explains the relationship between step size and data points - `max_points` parameter now includes guidance on when to increase (low-cardinality queries) vs decrease (overview graphs, high-cardinality queries) - **Added comprehensive test coverage**: New test class `TestMaxPointsOverride` with 5 test cases covering: - Override above default (allowed) - Override capped at hard limit - Override below default (allowed) - Invalid override fallback behavior - Interaction between explicit step and max_points override - **Updated Prometheus instructions**: Added new section on query resolution and data points with guidance on: - Default 500-point limit per series - When to increase max_points for spike/anomaly detection - Cardinality considerations - Alternative approach of narrowing time range for higher resolution ## Implementation Details - The hard limit is calculated as `MAX_GRAPH_POINTS * 2` to allow flexibility while maintaining safety - Invalid overrides (< 1) fall back to the default MAX_GRAPH_POINTS - When both step and max_points_override are provided, the step is adjusted if it would exceed the max_points target - Logging messages updated to reflect the new behavior and hard limits - All existing tests updated to use monkeypatch for MAX_GRAPH_POINTS to ensure test isolation https://claude.ai/code/session_0174s8zNS3peCtHc2tg9iJ8y <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Increased default maximum graph data points from 100 to 300. * Added a configurable hard limit (default 2× the max) that clamps and warns on excessive max-points. * **Improvements** * Step-size calculation now targets the configured data-point count; clearer messaging about limits and overrides. * Expanded parameter guidance for resolution vs. data points. * **Documentation** * Added query-resolution and data-point guidance with examples for Prometheus tools. * **Tests** * Added and updated tests covering step adjustments, overrides, and hard-limit behavior. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Claude <noreply@anthropic.com> Co-authored-by: Claude <noreply@anthropic.com> Signed-off-by: Mohse Morad <moshemorad12340@gmail.com>
…1566) ## Summary This PR improves Prometheus query resolution by increasing the default maximum data points from 100 to 500, and allowing the LLM to request up to 1000 points (2x the default) for high-resolution analysis of low-cardinality queries. The change includes updated logic, comprehensive tests, and improved documentation. ## Key Changes - **Increased default MAX_GRAPH_POINTS**: Changed from 100 to 500 to provide better default resolution for time series queries - **Implemented hard limit for max_points override**: LLM can now request up to 2x MAX_GRAPH_POINTS (1000 points) for higher resolution, with a hard cap to prevent excessive data retrieval - **Updated step calculation logic**: When no step is provided, the default now targets the configured max_points instead of a fixed 60-point target - **Enhanced parameter descriptions**: Updated tool parameter documentation to clarify: - `step` parameter now explains the relationship between step size and data points - `max_points` parameter now includes guidance on when to increase (low-cardinality queries) vs decrease (overview graphs, high-cardinality queries) - **Added comprehensive test coverage**: New test class `TestMaxPointsOverride` with 5 test cases covering: - Override above default (allowed) - Override capped at hard limit - Override below default (allowed) - Invalid override fallback behavior - Interaction between explicit step and max_points override - **Updated Prometheus instructions**: Added new section on query resolution and data points with guidance on: - Default 500-point limit per series - When to increase max_points for spike/anomaly detection - Cardinality considerations - Alternative approach of narrowing time range for higher resolution ## Implementation Details - The hard limit is calculated as `MAX_GRAPH_POINTS * 2` to allow flexibility while maintaining safety - Invalid overrides (< 1) fall back to the default MAX_GRAPH_POINTS - When both step and max_points_override are provided, the step is adjusted if it would exceed the max_points target - Logging messages updated to reflect the new behavior and hard limits - All existing tests updated to use monkeypatch for MAX_GRAPH_POINTS to ensure test isolation https://claude.ai/code/session_0174s8zNS3peCtHc2tg9iJ8y <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Increased default maximum graph data points from 100 to 300. * Added a configurable hard limit (default 2× the max) that clamps and warns on excessive max-points. * **Improvements** * Step-size calculation now targets the configured data-point count; clearer messaging about limits and overrides. * Expanded parameter guidance for resolution vs. data points. * **Documentation** * Added query-resolution and data-point guidance with examples for Prometheus tools. * **Tests** * Added and updated tests covering step adjustments, overrides, and hard-limit behavior. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Claude <noreply@anthropic.com> Co-authored-by: Claude <noreply@anthropic.com> Signed-off-by: Mohse Morad <moshemorad12340@gmail.com>
3 participants
Summary
This PR improves Prometheus query resolution by increasing the default maximum data points from 100 to 500, and allowing the LLM to request up to 1000 points (2x the default) for high-resolution analysis of low-cardinality queries. The change includes updated logic, comprehensive tests, and improved documentation.
Key Changes
stepparameter now explains the relationship between step size and data pointsmax_pointsparameter now includes guidance on when to increase (low-cardinality queries) vs decrease (overview graphs, high-cardinality queries)TestMaxPointsOverridewith 5 test cases covering:Implementation Details
MAX_GRAPH_POINTS * 2to allow flexibility while maintaining safetyhttps://claude.ai/code/session_0174s8zNS3peCtHc2tg9iJ8y
Summary by CodeRabbit
New Features
Improvements
Documentation
Tests