Repository navigation
Conversation
When the LLM marks all TodoWrite tasks as completed and includes its final answer text in the same response, we now return immediately instead of making an unnecessary extra LLM call. This saves 3-5 seconds per Holmes run. Changes: - Prompt updates across 4 template files instructing the LLM to batch its final answer with the last TodoWrite call - Code-level early return in both call() and call_stream() that detects all-tasks-completed and uses the existing text response https://claude.ai/code/session_01D3bToEw4CfTUkPWhVGsHCN 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 @ 60fd7b0 (#22129859294)✅ Results of HolmesGPT evalsAutomatically triggered by commit 60fd7b0 on branch Results of HolmesGPT evals
📜 Run @ 2582494 (#22097414575)✅ Results of HolmesGPT evalsAutomatically triggered by commit 2582494 on branch Results of HolmesGPT evals
📜 Run @ f0c4240 (#22079178276)✅ Results of HolmesGPT evalsAutomatically triggered by commit f0c4240 on branch Results of HolmesGPT evals
📜 Run @ b5d9cac (#22077454812)✅ Results of HolmesGPT evalsAutomatically triggered by commit b5d9cac on branch Results of HolmesGPT evals
📜 Run @ 9c7e80e (#22068355617)✅ Results of HolmesGPT evalsAutomatically triggered by commit 9c7e80e on branch Results of HolmesGPT evals
✅ Results of HolmesGPT evalsAutomatically triggered by commit c4dafba 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:344c262
docker tag us-central1-docker.pkg.dev/robusta-development/temporary-builds/holmes:344c262 me-west1-docker.pkg.dev/robusta-development/development/holmes-dev:344c262
docker push me-west1-docker.pkg.dev/robusta-development/development/holmes-dev:344c262Patch 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:344c262Robusta 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:344c262 |
WalkthroughUpdates four prompt templates to add an EFFICIENCY rule: when the last task is completed, the assistant should provide the final answer directly (embedding it in or replacing the final TodoWrite call), eliminating an extra TodoWrite/final-turn step. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Assistant
participant TodoWriteTool as TodoWrite
Note right of Assistant: New EFFICIENCY flow
User->>Assistant: Request / task list
Assistant->>TodoWrite: Create/Update tasks (multiple rounds)
TodoWrite-->>Assistant: Task statuses
alt Multiple tasks remain
Assistant->>TodoWrite: Mark task complete / create next task
TodoWrite-->>Assistant: Ack
else Last task completed
Assistant-->>User: Provide final answer (no final TodoWrite call)
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
🔬 CLI Performance Benchmark🟡 Startup Time (no LLM)Measures
🟡 Full CLI with LLMMeasures
PR: |
Remove the _all_todos_completed helper and early-return logic from both call() and call_stream() in tool_calling_llm.py. The prompt changes alone should be sufficient to get the LLM to batch its final answer with the last TodoWrite call, avoiding the extra LLM round trip. https://claude.ai/code/session_01D3bToEw4CfTUkPWhVGsHCN Signed-off-by: Claude <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@holmes/core/tool_calling_llm.py`:
- Around line 239-251: Update the type hint for the function
_all_todos_completed so tools_to_call is annotated as
list[ChatCompletionMessageToolCall] instead of the unparameterized list; locate
the _all_todos_completed definition and replace its parameter type accordingly
(using the already-imported ChatCompletionMessageToolCall) to satisfy complete
typing requirements.
🧹 Nitpick comments (1)
holmes/core/tool_calling_llm.py (1)
51-51: New cross-module import couplestool_calling_llmto the investigator toolset.Importing
TODO_WRITE_TOOL_NAMEfromholmes.plugins.toolsets.investigator.core_investigationcreates a dependency from the core module to a specific plugin/toolset. This is a minor architectural concern — if the investigator toolset is ever removed or restructured, this core module would break.Consider defining the constant in a shared location (e.g.,
holmes.core.constants) or passing it as configuration. Not blocking, but worth noting for future maintainability.
|
/eval |
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
Instead of asking the LLM to batch text with tool calls (unreliable), tell it to simply not call TodoWrite when the last task is done. The final answer itself signals completion, saving one full LLM round trip (3-5 seconds). https://claude.ai/code/session_01D3bToEw4CfTUkPWhVGsHCN Signed-off-by: Claude <noreply@anthropic.com>
…1564/git/HolmesGPT/holmesgpt into claude/fix-holmes-llm-call-RZFf6
|
/eval |
This comment was marked as outdated.
This comment was marked as outdated.
|
/eval |
|
@aantn Your eval run has finished. ✅ Completed successfully 🧪 Manual Eval Results
Results of HolmesGPT evals (branch:
|
| Status | Test case | Time | Turns | Tools | Cost |
|---|---|---|---|---|---|
| ✅ | 09_crashpod | 27.4s | 4 | 9 | $0.2002 |
| ✅ | 101_loki_historical_logs_pod_deleted | 47.8s | 5 | 10 | $0.2513 |
| ✅ | 111_pod_names_contain_service | 32.3s | 5 | 11 | $0.2225 |
| ✅ | 112_find_pvcs_by_uuid | 37.5s | 7 | 8 | $0.2490 |
| ✅ | 12_job_crashing | 57.4s | 5 | 12 | $0.2420 |
| ✅ | 176_network_policy_blocking_traffic_no_runbooks | 53.9s | 9 | 16 | $0.3268 |
| ✅ | 24_misconfigured_pvc | 33.2s | 5 | 13 | $0.2315 |
| ✅ | 43_current_datetime_from_prompt | 5.9s | 1 | — | $0.1064 |
| ✅ | 61_exact_match_counting | 13.6s | 3 | 2 | $0.1422 |
| Total | 34.3s avg | 4.9 avg | 10.1 avg | $1.9719 |
📖 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 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=
The "Final Review Phase" / "Investigation Verification" was a separate
tracked task that forced an extra TodoWrite round trip just to mark it
in_progress then completed. Convert it to a lightweight mental check
("Mentally verify, do NOT create a task for this") that the LLM does
before answering. This eliminates the extra LLM call the verification
task was causing.
https://claude.ai/code/session_01D3bToEw4CfTUkPWhVGsHCN
Signed-off-by: Claude <noreply@anthropic.com>
…6231/git/HolmesGPT/holmesgpt into claude/fix-holmes-llm-call-RZFf6
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/prompts/investigation_procedure.jinja2 (1)
52-77:⚠️ Potential issue | 🟠 MajorEnforcement rules at lines 54, 61, and 77 directly contradict the new Efficiency Rule — update the carve-out.
The new EFFICIENCY RULE (line 62) tells the LLM to skip the final TodoWrite completion call and provide the answer directly. However, the three unchanged enforcement statements directly block this path:
Line Statement Conflict 54 "Verify ALL tasks show 'completed' status"Last task never reaches completed61 "Only after ALL tasks are 'completed': Proceed to…final answer"Same — hard gate on all-completed 77 "If you see ANY [ ] pending or [~] in_progress tasks, DO NOT provide final answer"Last task is still in_progress; this check explicitly blocks the answerAn LLM trying to reconcile these will either silently fall back to the old behaviour (making the optimisation ineffective) or violate the enforcement rules unpredictably.
The enforcement block and the Task Status Check example (lines 71–77) should be updated to carve out the single-remaining-task exception, e.g.:
✏️ Suggested patch
-1. **Check TodoWrite status**: Verify ALL tasks show "completed" status -2. **If ANY task is "pending" or "in_progress"**: +1. **Check TodoWrite status**: Verify ALL tasks show "completed" status — **EXCEPT** when the single last task is in_progress (see EFFICIENCY RULE `#4` below). +2. **If ANY task is "pending" or "in_progress"** (and there is MORE THAN ONE remaining task):-If you see ANY `[ ] pending` or `[~] in_progress` tasks, DO NOT provide final answer. +If you see ANY `[ ] pending` or `[~] in_progress` tasks (and more than one task remains), DO NOT provide final answer. +If only ONE task is in_progress and you have completed the work, apply the EFFICIENCY RULE: provide your final answer directly.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@holmes/plugins/prompts/investigation_procedure.jinja2` around lines 52 - 77, Update the enforcement block in investigation_procedure.jinja2 to explicitly carve out the EFFICIENCY RULE exception: modify the three enforcement statements that require "ALL tasks show 'completed' status", "Only after ALL tasks are 'completed': Proceed...", and "If you see ANY [ ] pending or [~] in_progress tasks, DO NOT provide final answer" so they allow a single last-task exception when the EFFICIENCY RULE applies (i.e., if exactly one task remains and it is in_progress AND the agent has finished the work, the agent may provide the final answer instead of calling TodoWrite to mark it completed). Also update the "Task Status Check Example" block to reflect this exception (show an example where the last task is [~] in_progress but final answer is allowed under the EFFICIENCY RULE). Reference the enforcement block and the "Task Status Check Example" text in this template when making the edits.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@holmes/plugins/prompts/investigation_procedure.jinja2`:
- Around line 52-77: Update the enforcement block in
investigation_procedure.jinja2 to explicitly carve out the EFFICIENCY RULE
exception: modify the three enforcement statements that require "ALL tasks show
'completed' status", "Only after ALL tasks are 'completed': Proceed...", and "If
you see ANY [ ] pending or [~] in_progress tasks, DO NOT provide final answer"
so they allow a single last-task exception when the EFFICIENCY RULE applies
(i.e., if exactly one task remains and it is in_progress AND the agent has
finished the work, the agent may provide the final answer instead of calling
TodoWrite to mark it completed). Also update the "Task Status Check Example"
block to reflect this exception (show an example where the last task is [~]
in_progress but final answer is allowed under the EFFICIENCY RULE). Reference
the enforcement block and the "Task Status Check Example" text in this template
when making the edits.
This branch has not been deployed
2 participants
Summary
This PR optimizes the LLM interaction flow by allowing the model to provide its final answer in the same response as marking the last tasks as completed, eliminating unnecessary extra LLM calls.
Key Changes
_all_todos_completed()helper function to detect when all tasks in a TodoWrite call are marked as completedcall()andcall_stream()methods inToolCallingLLMto return early when:_general_instructions.jinja2_noflag_general_instructions.jinja2investigator_instructions.jinja2investigation_procedure.jinja2Implementation Details
The optimization works by:
The prompt updates make it clear to the LLM that it should include the final answer text in the same response as the final TodoWrite call, rather than waiting for a separate turn.
https://claude.ai/code/session_01D3bToEw4CfTUkPWhVGsHCN
Summary by CodeRabbit