Repository navigation
Conversation
…data) Two cases where the model could not express a larger query even when it explicitly asked for one: - fetch_datadog_logs silently clamped an explicit 'limit' down to the configured default_limit (min(requested, config)), so requesting more than 100 logs was impossible. The config value now acts as the default (matching its own description); explicit requests are honored up to the Datadog API page maximum of 1000, with the cursor for more pages. - The five Prometheus metadata APIs (get_metric_names, get_label_values, get_all_labels, get_series, get_metric_metadata) hardcoded limit=100 with no parameter at all; on truncation the model was only told to refine its filter. They now expose an optional 'limit' parameter (default still 100) and the _truncated message points at both recovery paths: raise the limit or refine the query. https://claude.ai/code/session_01BwJeGAGBLoby5rShhQADwq Signed-off-by: Claude <noreply@anthropic.com>
Contributor
📂 Previous Runs
|
| Status | Test case | Time | Turns | Tools | Cost | Total tokens | Input | Max input | Output | Max output | Cached | Non-cached | Reasoning | Compactions | Denied commands | Src |
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| ✅ | 09_crashpod | 28.6s | 4 | 8 | $0.2104 | 70,134 | 68,529 | 19,385 | 1,605 | 664 | 47,939 | 20,590 | 89 | — | — | src |
| ✅ | 101_loki_historical_logs_pod_deleted | 59.1s | 4 | 9 | $0.2408 | 71,381 | 68,936 | 21,019 | 2,445 | 741 | 47,364 | 21,572 | 202 | — | — | src |
| ✅ | 112_find_pvcs_by_uuid | 16.3s | 2 | 2 | $0.1483 | 32,073 | 31,164 | 16,842 | 909 | 546 | 14,319 | 16,845 | 153 | — | — | src |
| ✅ | 12_job_crashing | 40.0s | 5 | 11 | $0.2422 | 93,276 | 91,235 | 20,918 | 2,041 | 510 | 69,597 | 21,638 | 162 | — | — | src |
| ✅ | 176_network_policy_blocking_traffic_no_skills | 30.8s | 4 | 10 | $0.2253 | 70,352 | 68,584 | 21,049 | 1,768 | 548 | 45,989 | 22,595 | 287 | — | — | src |
| ✅ | 227_count_configmaps_per_namespace[0] | 15.4s | 3 | 6 | $0.1499 | 46,336 | 45,624 | 16,640 | 712 | 437 | 28,980 | 16,644 | 29 | — | — | src |
| ✅ | 243_pod_names_contain_service | 31.6s | 4 | 7 | $0.1957 | 67,106 | 65,577 | 18,139 | 1,529 | 494 | 46,860 | 18,717 | 197 | — | — | src |
| ✅ | 24_misconfigured_pvc | 29.8s | 4 | 9 | $0.2135 | 72,156 | 70,594 | 20,156 | 1,562 | 487 | 49,457 | 21,137 | 103 | — | — | src |
| ✅ | 254_elasticsearch_dr_test_log_check | 47.9s | 6 | 7 | $0.2037 | 72,580 | 69,795 | 14,026 | 2,785 | 1,500 | 55,762 | 14,033 | 449 | — | — | src |
| ✅ | 259_wrong_cluster_logs_confusion | 78.7s | 8 | 14 | $0.3299 | 123,215 | 118,338 | 19,580 | 4,877 | 1,544 | 97,374 | 20,964 | 878 | — | — | src |
| ✅ | 260_global_es_remote_cluster_logs | 64.9s | 9 | 14 | $0.3083 | 134,509 | 130,601 | 19,726 | 3,908 | 676 | 109,976 | 20,625 | 261 | — | — | src |
| ✅ | 43_current_datetime_from_prompt | 4.4s | 1 | — | $0.1016 | 14,424 | 14,306 | 14,306 | 118 | 118 | 0 | 14,306 | 74 | — | — | src |
| ✅ | 51_logs_summarize_errors | 20.4s | 3 | 2 | $0.1537 | 46,693 | 45,931 | 16,996 | 762 | 382 | 28,931 | 17,000 | 33 | — | — | src |
| ✅ | 61_exact_match_counting | 8.1s | 2 | 1 | $0.1146 | 29,149 | 28,928 | 14,642 | 221 | 152 | 14,283 | 14,645 | 34 | — | — | src |
| Total | 34.0s avg | 4.2 avg | 7.7 avg | $2.8381 | 943,384 | 918,142 | 21,049 | 25,242 | 1,544 | 656,831 | 261,311 | 2,951 | — | — |
Benchmark Comparison Details
Master baseline: latest master-* experiment (post-merge regression eval)
Status: 11 test/model combinations loaded
- master-27089078550 (created: 2026-06-07)
Benchmark baseline: latest ci-benchmark experiment on master
Status: 34 test/model combinations loaded
- ci-benchmark-27257681111 (created: 2026-06-10)
Time comparison (seconds):
| Test case | This branch | master (2d ago) | Δ vs master | benchmark (1h ago) | Δ vs benchmark |
|---|---|---|---|---|---|
| 09_crashpod (opus-4.6) 📄 | 28.6s | 26.2s | ±0% | — | — |
| 101_loki_historical_logs_pod_deleted (opus-4.6) 📄 | 59.1s | 47.6s | ↑24% | — | — |
| 112_find_pvcs_by_uuid (opus-4.6) 📄 | 16.3s | 10.6s | ↑54% | — | — |
| 12_job_crashing (opus-4.6) 📄 | 40.0s | 27.6s | ↑45% | — | — |
| 176_network_policy_blocking_traffic_no_skills (opus-4.6) 📄 | 30.8s | 32.6s | ±0% | — | — |
| 227_count_configmaps_per_namespace[0] (opus-4.6) 📄 | 15.4s | 13.2s | ↑16% | — | — |
| 243_pod_names_contain_service (opus-4.6) 📄 | 31.6s | 28.8s | ±0% | — | — |
| 24_misconfigured_pvc (opus-4.6) 📄 | 29.8s | 31.4s | ±0% | — | — |
| 254_elasticsearch_dr_test_log_check (opus-4.6) 📄 | 47.9s | — | — | — | — |
| 259_wrong_cluster_logs_confusion (opus-4.6) 📄 | 78.7s | — | — | — | — |
| 260_global_es_remote_cluster_logs (opus-4.6) 📄 | 64.9s | — | — | — | — |
| 43_current_datetime_from_prompt (opus-4.6) 📄 | 4.4s | 3.1s | ↑42% | — | — |
| 51_logs_summarize_errors (opus-4.6) 📄 | 20.4s | 18.8s | ±0% | — | — |
| 61_exact_match_counting (opus-4.6) 📄 | 8.1s | 7.1s | ↑13% | — | — |
| Total (all, n=14) | 34.0s | 22.5s | — | — | — |
| Comparable (m=11, b=0) | 25.9s | 22.5s | ↑15% | — | — |
Cost comparison:
| Test case | This branch | master (2d ago) | Δ vs master | benchmark (1h ago) | Δ vs benchmark |
|---|---|---|---|---|---|
| 09_crashpod (opus-4.6) 📄 | $0.2104 | $0.2032 | ±0% | — | — |
| 101_loki_historical_logs_pod_deleted (opus-4.6) 📄 | $0.2408 | $0.2642 | ±0% | — | — |
| 112_find_pvcs_by_uuid (opus-4.6) 📄 | $0.1483 | $0.1368 | ±0% | — | — |
| 12_job_crashing (opus-4.6) 📄 | $0.2422 | $0.2173 | ↑11% | — | — |
| 176_network_policy_blocking_traffic_no_skills (opus-4.6) 📄 | $0.2253 | $0.2524 | ↓11% | — | — |
| 227_count_configmaps_per_namespace[0] (opus-4.6) 📄 | $0.1499 | $0.1464 | ±0% | — | — |
| 243_pod_names_contain_service (opus-4.6) 📄 | $0.1957 | $0.1872 | ±0% | — | — |
| 24_misconfigured_pvc (opus-4.6) 📄 | $0.2135 | $0.2240 | ±0% | — | — |
| 254_elasticsearch_dr_test_log_check (opus-4.6) 📄 | $0.2037 | — | — | — | — |
| 259_wrong_cluster_logs_confusion (opus-4.6) 📄 | $0.3299 | — | — | — | — |
| 260_global_es_remote_cluster_logs (opus-4.6) 📄 | $0.3083 | — | — | — | — |
| 43_current_datetime_from_prompt (opus-4.6) 📄 | $0.1016 | $0.0110 | ↑825% | — | — |
| 51_logs_summarize_errors (opus-4.6) 📄 | $0.1537 | $0.1488 | ±0% | — | — |
| 61_exact_match_counting (opus-4.6) 📄 | $0.1146 | $0.1112 | ±0% | — | — |
| Total (all, n=14) | $0.2027 | $0.1730 | — | — | — |
| Comparable (m=11, b=0) | $0.1815 | $0.1730 | ±0% | — | — |
Total tokens comparison:
| Test case | This branch | master (2d ago) | Δ vs master | benchmark (1h ago) | Δ vs benchmark |
|---|---|---|---|---|---|
| 09_crashpod (opus-4.6) 📄 | 70,134 | 67,778 | ±0% | — | — |
| 101_loki_historical_logs_pod_deleted (opus-4.6) 📄 | 71,381 | 88,307 | ↓19% | — | — |
| 112_find_pvcs_by_uuid (opus-4.6) 📄 | 32,073 | 30,686 | ±0% | — | — |
| 12_job_crashing (opus-4.6) 📄 | 93,276 | 69,487 | ↑34% | — | — |
| 176_network_policy_blocking_traffic_no_skills (opus-4.6) 📄 | 70,352 | 108,726 | ↓35% | — | — |
| 227_count_configmaps_per_namespace[0] (opus-4.6) 📄 | 46,336 | 44,988 | ±0% | — | — |
| 243_pod_names_contain_service (opus-4.6) 📄 | 67,106 | 48,612 | ↑38% | — | — |
| 24_misconfigured_pvc (opus-4.6) 📄 | 72,156 | 68,923 | ±0% | — | — |
| 254_elasticsearch_dr_test_log_check (opus-4.6) 📄 | 72,580 | — | — | — | — |
| 259_wrong_cluster_logs_confusion (opus-4.6) 📄 | 123,215 | — | — | — | — |
| 260_global_es_remote_cluster_logs (opus-4.6) 📄 | 134,509 | — | — | — | — |
| 43_current_datetime_from_prompt (opus-4.6) 📄 | 14,424 | 13,970 | ±0% | — | — |
| 51_logs_summarize_errors (opus-4.6) 📄 | 46,693 | 45,163 | ±0% | — | — |
| 61_exact_match_counting (opus-4.6) 📄 | 29,149 | 28,231 | ±0% | — | — |
| Total (all, n=14) | 67,385 | 55,897 | — | — | — |
| Comparable (m=11, b=0) | 55,735 | 55,897 | ±0% | — | — |
Cached tokens comparison:
| Test case | This branch | master (2d ago) | Δ vs master | benchmark (1h ago) | Δ vs benchmark |
|---|---|---|---|---|---|
| 09_crashpod (opus-4.6) 📄 | 47,939 | 46,700 | ±0% | — | — |
| 101_loki_historical_logs_pod_deleted (opus-4.6) 📄 | 47,364 | 64,209 | ↓26% | — | — |
| 112_find_pvcs_by_uuid (opus-4.6) 📄 | 14,319 | 13,861 | ±0% | — | — |
| 12_job_crashing (opus-4.6) 📄 | 69,597 | 46,249 | ↑50% | — | — |
| 176_network_policy_blocking_traffic_no_skills (opus-4.6) 📄 | 45,989 | 84,274 | ↓45% | — | — |
| 227_count_configmaps_per_namespace[0] (opus-4.6) 📄 | 28,980 | 28,072 | ±0% | — | — |
| 243_pod_names_contain_service (opus-4.6) 📄 | 46,860 | 28,690 | ↑63% | — | — |
| 24_misconfigured_pvc (opus-4.6) 📄 | 49,457 | 46,110 | ±0% | — | — |
| 254_elasticsearch_dr_test_log_check (opus-4.6) 📄 | 55,762 | — | — | — | — |
| 259_wrong_cluster_logs_confusion (opus-4.6) 📄 | 97,374 | — | — | — | — |
| 260_global_es_remote_cluster_logs (opus-4.6) 📄 | 109,976 | — | — | — | — |
| 43_current_datetime_from_prompt (opus-4.6) 📄 | — | 13,845 | — | — | — |
| 51_logs_summarize_errors (opus-4.6) 📄 | 28,931 | 28,012 | ±0% | — | — |
| 61_exact_match_counting (opus-4.6) 📄 | 14,283 | 13,825 | ±0% | — | — |
| Total (all, n=14) | 46,916 | 37,622 | — | — | — |
| Comparable (m=10, b=0) | 39,372 | 40,000 | ±0% | — | — |
Turns comparison:
| Test case | This branch | master (2d ago) | Δ vs master | benchmark (1h ago) | Δ vs benchmark |
|---|---|---|---|---|---|
| 09_crashpod (opus-4.6) 📄 | 4 | 4 | ±0% | — | — |
| 101_loki_historical_logs_pod_deleted (opus-4.6) 📄 | 4 | 5 | ↓20% | — | — |
| 112_find_pvcs_by_uuid (opus-4.6) 📄 | 2 | 2 | ±0% | — | — |
| 12_job_crashing (opus-4.6) 📄 | 5 | 4 | ↑25% | — | — |
| 176_network_policy_blocking_traffic_no_skills (opus-4.6) 📄 | 4 | 6 | ↓33% | — | — |
| 227_count_configmaps_per_namespace[0] (opus-4.6) 📄 | 3 | 3 | ±0% | — | — |
| 243_pod_names_contain_service (opus-4.6) 📄 | 4 | 3 | ↑33% | — | — |
| 24_misconfigured_pvc (opus-4.6) 📄 | 4 | 4 | ±0% | — | — |
| 254_elasticsearch_dr_test_log_check (opus-4.6) 📄 | 6 | — | — | — | — |
| 259_wrong_cluster_logs_confusion (opus-4.6) 📄 | 8 | — | — | — | — |
| 260_global_es_remote_cluster_logs (opus-4.6) 📄 | 9 | — | — | — | — |
| 43_current_datetime_from_prompt (opus-4.6) 📄 | 1 | 1 | ±0% | — | — |
| 51_logs_summarize_errors (opus-4.6) 📄 | 3 | 3 | ±0% | — | — |
| 61_exact_match_counting (opus-4.6) 📄 | 2 | 2 | ±0% | — | — |
| Total (all, n=14) | 4.2 | 3.4 | — | — | — |
| Comparable (m=11, b=0) | 3.3 | 3.4 | ±0% | — | — |
Tool calls comparison:
| Test case | This branch | master (2d ago) | Δ vs master | benchmark (1h ago) | Δ vs benchmark |
|---|---|---|---|---|---|
| 09_crashpod (opus-4.6) 📄 | 8 | 8 | ±0% | — | — |
| 101_loki_historical_logs_pod_deleted (opus-4.6) 📄 | 9 | 11 | ↓18% | — | — |
| 112_find_pvcs_by_uuid (opus-4.6) 📄 | 2 | 2 | ±0% | — | — |
| 12_job_crashing (opus-4.6) 📄 | 11 | 9 | ↑22% | — | — |
| 176_network_policy_blocking_traffic_no_skills (opus-4.6) 📄 | 10 | 12 | ↓17% | — | — |
| 227_count_configmaps_per_namespace[0] (opus-4.6) 📄 | 6 | 6 | ±0% | — | — |
| 243_pod_names_contain_service (opus-4.6) 📄 | 7 | 7 | ±0% | — | — |
| 24_misconfigured_pvc (opus-4.6) 📄 | 9 | 13 | ↓31% | — | — |
| 254_elasticsearch_dr_test_log_check (opus-4.6) 📄 | 7 | — | — | — | — |
| 259_wrong_cluster_logs_confusion (opus-4.6) 📄 | 14 | — | — | — | — |
| 260_global_es_remote_cluster_logs (opus-4.6) 📄 | 14 | — | — | — | — |
| 43_current_datetime_from_prompt (opus-4.6) 📄 | — | — | — | — | — |
| 51_logs_summarize_errors (opus-4.6) 📄 | 2 | 2 | ±0% | — | — |
| 61_exact_match_counting (opus-4.6) 📄 | 1 | 1 | ±0% | — | — |
| Total (all, n=14) | 7.1 | 7.1 | — | — | — |
| Comparable (m=10, b=0) | 6.5 | 7.1 | ±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 claude/tool-limitations-honor-limits -f markers=regression -f filter=
Option 1: Comment on this PR with /eval:
/eval
tags: regression
Or with more options (one per line):
/eval
model: gpt-4o
tags: regression
id: 09_crashpod
iterations: 5
Run evals on a different branch (e.g., master) for comparison:
/eval
branch: master
tags: regression
| Option | Description |
|---|---|
model |
Model(s) to test (default: same as automatic runs) |
tags |
Pytest tags / markers (no default - runs all tests!) |
id |
Eval ID / pytest -k filter (use /list to see valid eval names) |
iterations |
Number of runs, max 10 |
branch |
Run evals on a different branch (for cross-branch comparison) |
Quick re-run: Use /rerun to re-run the most recent /eval on this PR with the same parameters.
Option 2: Trigger via GitHub Actions UI → "Run workflow"
Option 3: Add PR labels to include extra evals (applies to both automatic runs and /eval comments):
| Label | Effect |
|---|---|
evals-tag-<name> |
Run tests with tag <name> alongside regression |
evals-id-<name> |
Run a specific eval by test ID |
evals-model-<name> |
Override the model (use model list name, e.g. sonnet-4.5) |
Examples: evals-tag-easy, evals-id-09_crashpod, evals-model-sonnet-4.5
🏷️ Valid tags
benchmark, chain-of-causation, compaction, confluence, context_window, conversation_worker, coralogix, counting, database, datadog, datetime, db-connectors, easy, elasticsearch, embeds, fast, frontend, grafana, hard, images, integration, kafka, kubernetes, leaked-information, logs, loki, manual, mcp, medium, metrics, multi-cluster, network, newrelic, no-cicd, numerical, one-test, port-forward, prometheus, question-answer, regression, skills, slackbot, storage, token-limit, toolset-limitation, traces, transparency, victorialogs
🤖 Valid models
deepseek-chat, deepseek-r1-reasoner, deepseek-reasoner, deepseek-v3.2-chat, gemini-3-flash-preview, gemini-3-pro-preview, gemini-3.1-pro-preview, gpt-4.1, gpt-5.2-high-reasoning, gpt-5.3-codex, gpt-5.4, gpt-5.5, haiku-4.5, kimi-2.5, kimi-2.5-openrouter, opus-4.5, opus-4.6, opus-4.7, opus-4.8, 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/tool-limitations-honor-limits -f markers=regression -f filter=
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (3)
WalkthroughThis PR standardizes pagination limits: Datadog logs honor explicit limits (clamped to API max) while Prometheus metadata tools gain shared helpers to accept, normalize, and mark truncated results based on a requested limit. ChangesPagination Limit Control
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:9bfd8e52b
docker tag us-central1-docker.pkg.dev/robusta-development/temporary-builds/holmes:9bfd8e52b me-west1-docker.pkg.dev/robusta-development/development/holmes-dev:9bfd8e52b
docker push me-west1-docker.pkg.dev/robusta-development/development/holmes-dev:9bfd8e52b
docker pull us-central1-docker.pkg.dev/robusta-development/temporary-builds/holmes-operator:9bfd8e52b
docker tag us-central1-docker.pkg.dev/robusta-development/temporary-builds/holmes-operator:9bfd8e52b me-west1-docker.pkg.dev/robusta-development/development/holmes-operator-dev:9bfd8e52b
docker push me-west1-docker.pkg.dev/robusta-development/development/holmes-operator-dev:9bfd8e52bPatch 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:9bfd8e52b \
--set operator.registry=me-west1-docker.pkg.dev/robusta-development/development \
--set operator.image=holmes-operator-dev:9bfd8e52bRobusta 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:9bfd8e52b \
--set holmes.operator.registry=me-west1-docker.pkg.dev/robusta-development/development \
--set holmes.operator.image=holmes-operator-dev:9bfd8e52b |
![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: 1
🧹 Nitpick comments (1)
holmes/plugins/toolsets/datadog/toolset_datadog_logs.py (1)
223-228: 💤 Low valueConsider using explicit None-check for more predictable default handling.
The expression
params.get("limit") or self.toolset.dd_config.default_limittreats all falsy values (including0) as "not provided" and falls back to the default. Whilelimit=0is likely invalid for pagination and the current idiom is common in Python, an explicit None-check would be clearer and more robust:limit = params.get("limit") if params.get("limit") is not None else self.toolset.dd_config.default_limitor
limit = params.get("limit") if limit is None: limit = self.toolset.dd_config.default_limitThis avoids the edge case where an explicit
0is mistaken for "use default," though in practice the Datadog API would reject0anyway.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@holmes/plugins/toolsets/datadog/toolset_datadog_logs.py` around lines 223 - 228, The current assignment for limit uses a falsy check which treats 0 and other falsy values as "not provided"; change it to perform an explicit None-check on params.get("limit") so an explicitly provided 0 (or other falsy value) is preserved rather than replaced by self.toolset.dd_config.default_limit, then clamp the resulting limit with DATADOG_LOGS_API_PAGE_MAX; locate the code around the params.get("limit") assignment in this module (the variable name limit, params, self.toolset.dd_config.default_limit, and DATADOG_LOGS_API_PAGE_MAX) and replace the idiomatic or-based fallback with a None-aware fallback.
🤖 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/prometheus/prometheus.py`:
- Around line 1037-1038: The code is incorrectly using "or" so an explicit
limit=0 gets replaced with PROMETHEUS_METADATA_API_LIMIT; change the logic to
treat only None as missing: fetch raw = params.get("limit"), if raw is None set
limit = PROMETHEUS_METADATA_API_LIMIT, else set limit = int(raw), then return
max(1, limit). Update the function that reads params (the block that assigns to
variable limit and returns max(1, int(limit))) to use this None-check, and add a
unit test asserting that passing {"limit": 0} yields a result of 1 to lock the
behavior.
---
Nitpick comments:
In `@holmes/plugins/toolsets/datadog/toolset_datadog_logs.py`:
- Around line 223-228: The current assignment for limit uses a falsy check which
treats 0 and other falsy values as "not provided"; change it to perform an
explicit None-check on params.get("limit") so an explicitly provided 0 (or other
falsy value) is preserved rather than replaced by
self.toolset.dd_config.default_limit, then clamp the resulting limit with
DATADOG_LOGS_API_PAGE_MAX; locate the code around the params.get("limit")
assignment in this module (the variable name limit, params,
self.toolset.dd_config.default_limit, and DATADOG_LOGS_API_PAGE_MAX) and replace
the idiomatic or-based fallback with a None-aware fallback.
🪄 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: 4cf302c3-b89b-4383-b75e-4e3472d36f52
📒 Files selected for processing (4)
holmes/plugins/toolsets/datadog/toolset_datadog_logs.pyholmes/plugins/toolsets/prometheus/prometheus.pytests/plugins/toolsets/datadog/logs/test_fetch_logs_limit.pytests/plugins/toolsets/test_prometheus_unit.py
CodeRabbit review: 'params.get("limit") or default' silently replaced an
explicit limit=0 with the default. Use None-checks in get_metadata_limit
and fetch_datadog_logs so explicit values are honored (clamped to >= 1),
and lock the behavior with tests.
https://claude.ai/code/session_01BwJeGAGBLoby5rShhQADwq
Signed-off-by: Claude <noreply@anthropic.com>
https://claude.ai/code/session_01BwJeGAGBLoby5rShhQADwq 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. |
✅ Deploy Preview for holmes-docs ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for holmes-docs ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
aantn
added a commit
that referenced
this pull request
Jun 10, 2026
Documentation-only change, no behavior change. The `llm_summarize` transformer predates the spill-to-disk mechanism, is disabled by default, and never worked well in practice: summarization is lossy (the original tool output is unrecoverable afterwards) and it adds latency and cost to every large tool call. Modern models do better working from the full data spilled to disk. This marks it as legacy in the module/class docstrings and adds a warning admonition to `docs/development/transformers.md`, so future contributors don't build on it and users don't enable it expecting good results. Kept for backwards compatibility with existing configs that reference it. Part of a series of targeted fixes removing old-model data limitations from built-in tools (see #2167, #2168, #2169). https://claude.ai/code/session_01BwJeGAGBLoby5rShhQADwq --- _Generated by [Claude Code](https://claude.ai/code/session_01BwJeGAGBLoby5rShhQADwq)_ <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Documentation** * Added notices marking the `llm_summarize` transformer as a legacy feature that is disabled by default. This feature is not recommended for new configurations due to lossy summarization, increased latency, and associated costs. Documentation has been updated to recommend the spill-to-disk mechanism as the preferred alternative for handling oversized tool results. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Claude <noreply@anthropic.com> Co-authored-by: Claude <noreply@anthropic.com>
This branch has not been deployed
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.
Problem
Two places where the model literally could not express a larger query, even when it explicitly asked for one. These were designed for older models that couldn't be trusted with limit parameters:
fetch_datadog_logs:limit = min(params.get("limit", config_limit), config_limit)— a model requestinglimit: 500was silently clamped to 100. The config field even describes itself as "Default maximum number of log events to return when a limit is not explicitly provided", but the code enforced it as a hard ceiling.get_metric_names,get_label_values,get_all_labels,get_series,get_metric_metadata): hardcodedlimit=100with no parameter at all. On truncation the model was told only to "use a more specific match filter" — there was no way to just see the full list.Changes
default_limitnow acts as the default; an explicit model-suppliedlimitis honored up toDATADOG_LOGS_API_PAGE_MAX(1000, the Datadog API page maximum), with the existing cursor for further pages. Parameter description updated to match.limitparameter (default unchanged at 100). The repeated truncation-detection blocks are factored intomark_metadata_truncation(), and the_truncatedmessage now names both recovery paths: raiselimit, or refine the filter. Tool descriptions updated accordingly.Testing
responses) assertingpage.limitsent to Datadog honors explicit requests, defaults correctly, and caps at the API max.get_metadata_limit/mark_metadata_truncation(list + dict result shapes) and that all five metadata tools exposelimit.Part of a series of targeted fixes removing old-model data limitations from built-in tools (see #2167, #2168).
Note:
database/mongodbmax_rowswas deliberately left as a ceiling — it's an explicit user-configured guardrail against expensive queries on production databases, unlike these two cases where the cap was an accident of old-model defensiveness.https://claude.ai/code/session_01BwJeGAGBLoby5rShhQADwq
Generated by Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Tests