Skip to content

Load config from env override - #2099

Closed
naomi-robusta wants to merge 2 commits into
masterfrom
load_config_from_env_override
Closed

naomi-robusta wants to merge 2 commits into
masterfrom
load_config_from_env_override

Conversation

@naomi-robusta

@naomi-robusta naomi-robusta commented May 28, 2026 •

Copy link
Copy Markdown
Collaborator

Summary by CodeRabbit

  • Bug Fixes

    • Fixed configuration loading to respect the LOAD_CONFIG_FROM_ENV environment variable flag.
    • Improved handling of abandoned tool requests by automatically injecting error responses to maintain proper message sequencing.
  • Tests

    • Added test coverage for orphaned tool call resolution scenarios.

Review Change Stack

claude and others added 2 commits May 27, 2026 11:30
When the LLM requested a tool call requiring approval and the user
abandoned it (closed the approval modal or asked a new follow-up
question without deciding), the conversation history ended up with an
assistant tool_calls message that had no matching tool result. The next
LLM call then failed with "tool_use ids were found without tool_result
blocks immediately after" on Anthropic/Bedrock.

call_stream now resolves any orphaned tool calls by injecting a denial
tool result, so the conversation can continue with the user's new
request.

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

@claude claude 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.

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.

@naomi-robusta
naomi-robusta requested a review from moshemorad May 28, 2026 09:21
@coderabbitai

coderabbitai Bot commented May 28, 2026 •

Copy link
Copy Markdown
Contributor

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: df064842-2b80-4784-ba57-acb2435a099e

📥 Commits

Reviewing files that changed from the base of the PR and between 0b12d31 and 570b236.

📒 Files selected for processing (3)
  • holmes/core/tool_calling_llm.py
  • server.py
  • tests/test_orphaned_tool_calls.py

Walkthrough

The PR adds orphaned assistant tool call resolution to ToolCallingLLM by detecting unresolved tool_calls, injecting denial tool messages with corresponding TOOL_RESULT events, and integrating this into call_stream. A separate, unrelated change updates server config loading to respect the LOAD_CONFIG_FROM_ENV environment variable.

Changes

Orphaned Tool Call Resolution

Layer / File(s) Summary
Orphaned tool resolution implementation and call_stream integration
holmes/core/tool_calling_llm.py
_resolve_orphaned_tool_calls() detects assistant tool_calls without matching tool_result messages, removes stale pending_approval flags, injects error ToolCallResult messages in the correct position after the assistant message, and emits TOOL_RESULT stream events. call_stream() invokes this before normal iteration, yielding injected events to preserve provider-required tool_use→tool_result ordering.
Orphaned tool call test coverage
tests/test_orphaned_tool_calls.py
Test module with setup helpers and four tests verifying: denial insertion when pending_approval is present, denial insertion when absent, unmodified handling of already-resolved calls with no events, and multi-call scenarios where each orphaned call receives a denial result and corresponding event.

Configuration Loading

Layer / File(s) Summary
Environment variable config loading condition
server.py
init_config() now skips default config file loading when LOAD_CONFIG_FROM_ENV environment variable is "false", forcing environment-variable-based configuration even if the default config file exists.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related PRs

  • HolmesGPT/holmesgpt#2097: Adds identical ToolCallingLLM._resolve_orphaned_tool_calls logic and tests/test_orphaned_tool_calls.py test coverage for injecting denial tool results when assistant tool calls are orphaned.
  • HolmesGPT/holmesgpt#1054: Modifies ToolCallingLLM in holmes/core/tool_calling_llm.py to generate denial ToolCallResult responses and emit TOOL_RESULT stream events for unresolved tool calls.
  • HolmesGPT/holmesgpt#1430: Modifies server.py's init_config() logic for configuration loading from file versus environment variables.

Suggested reviewers

  • moshemorad
  • arikalon1

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.

@github-actions

github-actions Bot commented May 28, 2026 •

Copy link
Copy Markdown
Contributor

✅ Results of HolmesGPT evals

Automatically triggered by commit 570b236 on branch load_config_from_env_override

View workflow logs

Results of HolmesGPT evals

  • ask_holmes: 11/11 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 Src
✅ 09_crashpod 52.3s 8 14 $0.3410 181,243 178,370 25,450 2,873 840 152,339 26,031 250 — src
✅ 101_loki_historical_logs_pod_deleted 69.2s 7 17 $0.4132 173,031 168,820 29,240 4,211 918 135,621 33,199 678 — src
✅ 112_find_pvcs_by_uuid 20.9s 3 4 $0.2068 61,222 59,982 21,887 1,240 666 37,839 22,143 359 — src
✅ 12_job_crashing 41.4s 6 13 $0.3115 134,139 131,649 24,972 2,490 875 104,113 27,536 188 — src
✅ 176_network_policy_blocking_traffic_no_skills 49.0s 5 15 $0.3116 112,019 108,852 25,300 3,167 940 82,620 26,232 423 — src
✅ 227_count_configmaps_per_namespace[0] 21.1s 4 9 $0.2030 76,653 75,528 20,638 1,125 592 54,885 20,643 53 — src
✅ 243_pod_names_contain_service 36.9s 5 9 $0.2548 101,639 99,727 22,449 1,912 629 76,193 23,534 166 — src
✅ 24_misconfigured_pvc 46.3s 6 14 $0.3093 132,655 129,855 24,610 2,800 954 104,194 25,661 213 — src
✅ 43_current_datetime_from_prompt 3.8s 1 — $0.1190 17,001 16,899 16,899 102 102 0 16,899 61 — src
✅ 51_logs_summarize_errors 22.8s 4 5 $0.2049 77,752 76,726 21,265 1,026 373 55,456 21,270 32 — src
✅ 61_exact_match_counting 12.2s 3 3 $0.1518 52,717 52,356 17,870 361 214 34,482 17,874 31 — src
Total 34.2s avg 4.7 avg 10.3 avg $2.8269 1,120,071 1,098,764 29,240 21,307 954 837,742 261,022 2,454 —
Benchmark Comparison Details

Master baseline: latest master-* experiment (post-merge regression eval)
Status: No eval spans found in experiment 'master-26509361386'

Benchmark baseline: latest ci-benchmark experiment on master
Status: 187 test/model combinations loaded

Time comparison (seconds):

Test case This branch master (21h ago) Δ vs master benchmark (3d ago) Δ vs benchmark
09_crashpod (opus-4.6) 📄 52.3s — — 40.5s ↑29%
101_loki_historical_logs_pod_deleted (opus-4.6) 📄 69.2s — — 576.6s ↓88%
112_find_pvcs_by_uuid (opus-4.6) 📄 20.9s — — 20.2s ±0%
12_job_crashing (opus-4.6) 📄 41.4s — — 53.5s ↓23%
176_network_policy_blocking_traffic_no_skills (opus-4.6) 📄 49.0s — — 59.6s ↓18%
227_count_configmaps_per_namespace[0] (opus-4.6) 📄 21.1s — — 19.1s ↑10%
243_pod_names_contain_service (opus-4.6) 📄 36.9s — — 33.1s ↑12%
24_misconfigured_pvc (opus-4.6) 📄 46.3s — — 43.5s ±0%
43_current_datetime_from_prompt (opus-4.6) 📄 3.8s — — 3.8s ±0%
51_logs_summarize_errors (opus-4.6) 📄 22.8s — — 21.2s ±0%
61_exact_match_counting (opus-4.6) 📄 12.2s — — 10.3s ↑19%
Total (all, n=11) 34.2s — — 80.1s —
Comparable (m=0, b=11) 34.2s — — 80.1s ↓57%

Cost comparison:

Test case This branch master (21h ago) Δ vs master benchmark (3d ago) Δ vs benchmark
09_crashpod (opus-4.6) 📄 $0.3410 — — $0.3040 ↑12%
101_loki_historical_logs_pod_deleted (opus-4.6) 📄 $0.4132 — — $2.8482 ↓85%
112_find_pvcs_by_uuid (opus-4.6) 📄 $0.2068 — — $0.2053 ±0%
12_job_crashing (opus-4.6) 📄 $0.3115 — — $0.3395 ±0%
176_network_policy_blocking_traffic_no_skills (opus-4.6) 📄 $0.3116 — — $0.3688 ↓16%
227_count_configmaps_per_namespace[0] (opus-4.6) 📄 $0.2030 — — $0.2039 ±0%
243_pod_names_contain_service (opus-4.6) 📄 $0.2548 — — $0.2499 ±0%
24_misconfigured_pvc (opus-4.6) 📄 $0.3093 — — $0.3133 ±0%
43_current_datetime_from_prompt (opus-4.6) 📄 $0.1190 — — $0.1183 ±0%
51_logs_summarize_errors (opus-4.6) 📄 $0.2049 — — $0.2030 ±0%
61_exact_match_counting (opus-4.6) 📄 $0.1518 — — $0.1510 ±0%
Total (all, n=11) $0.2570 — — $0.4823 —
Comparable (m=0, b=11) $0.2570 — — $0.4823 ↓47%

Total tokens comparison:

Test case This branch master (21h ago) Δ vs master benchmark (3d ago) Δ vs benchmark
09_crashpod (opus-4.6) 📄 181,243 — — 130,696 ↑39%
101_loki_historical_logs_pod_deleted (opus-4.6) 📄 173,031 — — 2,481,402 ↓93%
112_find_pvcs_by_uuid (opus-4.6) 📄 61,222 — — 60,840 ±0%
12_job_crashing (opus-4.6) 📄 134,139 — — 159,134 ↓16%
176_network_policy_blocking_traffic_no_skills (opus-4.6) 📄 112,019 — — 145,397 ↓23%
227_count_configmaps_per_namespace[0] (opus-4.6) 📄 76,653 — — 76,285 ±0%
243_pod_names_contain_service (opus-4.6) 📄 101,639 — — 81,620 ↑25%
24_misconfigured_pvc (opus-4.6) 📄 132,655 — — 132,763 ±0%
43_current_datetime_from_prompt (opus-4.6) 📄 17,001 — — 16,900 ±0%
51_logs_summarize_errors (opus-4.6) 📄 77,752 — — 76,834 ±0%
61_exact_match_counting (opus-4.6) 📄 52,717 — — 52,421 ±0%
Total (all, n=11) 101,825 — — 310,390 —
Comparable (m=0, b=11) 101,825 — — 310,390 ↓67%

Cached tokens comparison:

Test case This branch master (21h ago) Δ vs master benchmark (3d ago) Δ vs benchmark
09_crashpod (opus-4.6) 📄 152,339 — — 102,310 ↑49%
101_loki_historical_logs_pod_deleted (opus-4.6) 📄 135,621 — — 2,330,970 ↓94%
112_find_pvcs_by_uuid (opus-4.6) 📄 37,839 — — 37,594 ±0%
12_job_crashing (opus-4.6) 📄 104,113 — — 129,152 ↓19%
176_network_policy_blocking_traffic_no_skills (opus-4.6) 📄 82,620 — — 111,403 ↓26%
227_count_configmaps_per_namespace[0] (opus-4.6) 📄 54,885 — — 54,286 ±0%
243_pod_names_contain_service (opus-4.6) 📄 76,193 — — 56,147 ↑36%
24_misconfigured_pvc (opus-4.6) 📄 104,194 — — 104,046 ±0%
43_current_datetime_from_prompt (opus-4.6) 📄 — — — — —
51_logs_summarize_errors (opus-4.6) 📄 55,456 — — 54,880 ±0%
61_exact_match_counting (opus-4.6) 📄 34,482 — — 34,283 ±0%
Total (all, n=11) 76,158 — — 274,097 —
Comparable (m=0, b=10) 83,774 — — 301,507 ↓72%

Turns comparison:

Test case This branch master (21h ago) Δ vs master benchmark (3d ago) Δ vs benchmark
09_crashpod (opus-4.6) 📄 8 — — 6 ↑33%
101_loki_historical_logs_pod_deleted (opus-4.6) 📄 7 — — 41 ↓83%
112_find_pvcs_by_uuid (opus-4.6) 📄 3 — — 3 ±0%
12_job_crashing (opus-4.6) 📄 6 — — 7 ↓14%
176_network_policy_blocking_traffic_no_skills (opus-4.6) 📄 5 — — 6 ↓17%
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 — — 6 ±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.7 — — 7.7 —
Comparable (m=0, b=11) 4.7 — — 7.7 ↓39%

Tool calls comparison:

Test case This branch master (21h ago) Δ vs master benchmark (3d ago) Δ vs benchmark
09_crashpod (opus-4.6) 📄 14 — — 13 ±0%
101_loki_historical_logs_pod_deleted (opus-4.6) 📄 17 — — 98 ↓83%
112_find_pvcs_by_uuid (opus-4.6) 📄 4 — — 4 ±0%
12_job_crashing (opus-4.6) 📄 13 — — 16 ↓19%
176_network_policy_blocking_traffic_no_skills (opus-4.6) 📄 15 — — 15 ±0%
227_count_configmaps_per_namespace[0] (opus-4.6) 📄 9 — — 9 ±0%
243_pod_names_contain_service (opus-4.6) 📄 9 — — 10 ↓10%
24_misconfigured_pvc (opus-4.6) 📄 14 — — 15 ±0%
43_current_datetime_from_prompt (opus-4.6) 📄 — — — — —
51_logs_summarize_errors (opus-4.6) 📄 5 — — 5 ±0%
61_exact_match_counting (opus-4.6) 📄 3 — — 3 ±0%
Total (all, n=11) 9.4 — — 18.8 —
Comparable (m=0, b=10) 10.3 — — 18.8 ↓45%

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 load_config_from_env_override -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 load_config_from_env_override -f markers=regression -f filter=

@naomi-robusta
naomi-robusta removed the request for review from moshemorad May 28, 2026 09:21
@github-actions

github-actions Bot commented May 28, 2026 •

Copy link
Copy Markdown
Contributor

✅ Docker images ready for 1ef054da (built in 5m 48s)

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

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

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

@netlify

netlify Bot commented May 28, 2026

Copy link
Copy Markdown

✅ Deploy Preview for holmes-docs ready!

Name Link
🔨 Latest commit 570b236
🔍 Latest deploy log https://app.netlify.com/projects/holmes-docs/deploys/6a18090eaccded00094a2b27
😎 Deploy Preview https://deploy-preview-2099--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.

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.

2 participants