Skip to content

Move MANDATORY Task Management block back to its pre-#1970 position - #2057

Open
aantn wants to merge 1 commit into
masterfrom
claude/fix-eval-perf-task-mgmt-position
Open

aantn wants to merge 1 commit into
masterfrom
claude/fix-eval-perf-task-mgmt-position

Conversation

@aantn

@aantn aantn commented May 17, 2026 •

Copy link
Copy Markdown
Collaborator

PR #1970 inlined investigation_procedure.jinja2 into generic_ask.jinja2 and, in doing so, repositioned the # MANDATORY Task Management block from its old slot (between # Special cases and how to reply and # Tool/function calls, near the end of the prompt) to immediately after # INVESTIGATION PHASE TRANSITION EXAMPLES, in the middle of the prompt.

The block content is byte-identical; only its position changed. But that single positional move turns out to drive the residual ~10% eval-perf regression that survived PR #2051's revert of PR #2040.

Local n=10 sweep against the docker-loki regression eval (#259) on opus-4.6:

condition time turns total_tk compl_tk
baseline (cf6ddb7) 78.6s 7.8 193,919 5,097
PR-merged (fix-AD only) 83.0s 8.6 202,756 4,834 <- residual
Restore deleted intro lines 76.0s 9.0 208,022 4,647
THIS PATCH (just block move) 71.1s 7.6 182,068 4,421
Block move + intro lines 76.1s 7.6 185,765 4,829
Full baseline prompt 75.1s 7.2 182,008 4,914

Every z vs baseline for this patch is ≤ 0: time z=-1.91, turns z=-0.46, completion z=-2.14, total tokens z=-0.92. The block-move alone is strictly better than restoring the two deleted intro lines, and matches the full baseline-prompt restoration on aggregate metrics. No text added or removed; just position.

Hypothesis on the mechanism: with the task-management rules sandwiched inside the multi-phase investigation doctrine, the model reads "If you discover additional steps during investigation, add them to your task list using TodoWrite" while it is still planning phases and acts on it more aggressively (extra TodoWrite mid-investigation, extra turns). When the same rules sit at the end of the prompt as a quiet reminder (post-Special-Cases, pre-Tool/function-calls), they don't compound with the phase-planning instructions.

Summary by CodeRabbit

  • Chores
    • Reorganized task management prompt instructions for improved template structure.

Review Change Stack

PR #1970 inlined investigation_procedure.jinja2 into generic_ask.jinja2 and,
in doing so, repositioned the `# MANDATORY Task Management` block from its
old slot (between `# Special cases and how to reply` and
`# Tool/function calls`, near the end of the prompt) to immediately after
`# INVESTIGATION PHASE TRANSITION EXAMPLES`, in the middle of the prompt.

The block content is byte-identical; only its position changed. But that
single positional move turns out to drive the residual ~10% eval-perf
regression that survived PR #2051's revert of PR #2040.

Local n=10 sweep against the docker-loki regression eval (#259) on opus-4.6:

  condition                       time    turns  total_tk   compl_tk
  baseline (cf6ddb7)              78.6s    7.8   193,919     5,097
  PR-merged (fix-AD only)         83.0s    8.6   202,756     4,834   <- residual
  Restore deleted intro lines     76.0s    9.0   208,022     4,647
  THIS PATCH (just block move)    71.1s    7.6   182,068     4,421
  Block move + intro lines        76.1s    7.6   185,765     4,829
  Full baseline prompt            75.1s    7.2   182,008     4,914

Every z vs baseline for this patch is ≤ 0: time z=-1.91, turns z=-0.46,
completion z=-2.14, total tokens z=-0.92. The block-move alone is strictly
better than restoring the two deleted intro lines, and matches the full
baseline-prompt restoration on aggregate metrics. No text added or
removed; just position.

Hypothesis on the mechanism: with the task-management rules sandwiched
inside the multi-phase investigation doctrine, the model reads "If you
discover additional steps during investigation, add them to your task
list using TodoWrite" while it is still planning phases and acts on it
more aggressively (extra TodoWrite mid-investigation, extra turns).
When the same rules sit at the end of the prompt as a quiet reminder
(post-Special-Cases, pre-Tool/function-calls), they don't compound with
the phase-planning instructions.

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.

@coderabbitai

coderabbitai Bot commented May 17, 2026 •

Copy link
Copy Markdown
Contributor

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 80c64c9e-82f6-438d-990a-3d8b21d11602

📥 Commits

Reviewing files that changed from the base of the PR and between fa5e0ce and f434e61.

📒 Files selected for processing (1)
  • holmes/plugins/prompts/generic_ask.jinja2

Walkthrough

This PR relocates the "MANDATORY Task Management" instruction subsection within a Jinja2 prompt template. The block is removed from its original position and re-added later under a new conditional, preserving the complete instruction set for TodoWrite task management.

Changes

Task Management Instructions Relocation

Layer / File(s) Summary
Reorganize Task Management instructions in todowrite prompt
holmes/plugins/prompts/generic_ask.jinja2
The "MANDATORY Task Management" subsection is moved from lines 234–235 to lines 299–315, relocating instructions on TodoWrite plan creation, task status updates, and parallel tool calls to a new {% if todowrite_enabled %} block while preserving content.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~5 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: relocating the MANDATORY Task Management block to its original position, which is the primary purpose of this PR.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ 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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions

github-actions Bot commented May 17, 2026 •

Copy link
Copy Markdown
Contributor

✅ Results of HolmesGPT evals

Automatically triggered by commit f434e61 on branch claude/fix-eval-perf-task-mgmt-position

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 31.0s 4 8 $0.2453 82,285 80,374 22,768 1,911 771 56,697 23,677 223 — src
✅ 101_loki_historical_logs_pod_deleted 85.3s 8 19 $0.4582 202,574 197,156 30,323 5,418 961 165,018 32,138 1,155 — src
✅ 112_find_pvcs_by_uuid 21.2s 3 4 $0.2066 60,962 59,700 21,794 1,262 676 37,651 22,049 365 — src
✅ 12_job_crashing 45.8s 7 15 $0.3287 159,498 156,743 25,569 2,755 856 130,171 26,572 189 — src
✅ 176_network_policy_blocking_traffic_no_skills 58.1s 7 18 $0.3826 168,506 164,700 28,465 3,806 810 134,749 29,951 638 — src
✅ 227_count_configmaps_per_namespace[0] 20.7s 4 9 $0.2083 76,261 75,136 20,543 1,125 591 53,360 21,776 53 — src
✅ 243_pod_names_contain_service 36.0s 5 9 $0.2602 102,502 100,358 22,634 2,144 913 77,148 23,210 223 — src
✅ 24_misconfigured_pvc 40.9s 5 14 $0.2869 107,438 104,857 23,980 2,581 950 79,548 25,309 218 — src
✅ 43_current_datetime_from_prompt 4.0s 1 — $0.1176 16,876 16,798 16,798 78 78 0 16,798 38 — src
✅ 51_logs_summarize_errors 22.1s 4 5 $0.2077 77,659 76,529 21,260 1,130 400 55,264 21,265 45 — src
✅ 61_exact_match_counting 12.0s 3 3 $0.1510 52,415 52,053 17,770 362 215 34,279 17,774 31 — src
Total 34.3s avg 4.6 avg 10.4 avg $2.8531 1,106,976 1,084,404 30,323 22,572 961 823,885 260,519 3,178 —
Benchmark Comparison Details

Master baseline: latest master-* experiment (post-merge regression eval)
Status: 11 test/model combinations loaded

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

Time comparison (seconds):

Test case This branch master (8h ago) Δ vs master benchmark (14d ago) Δ vs benchmark
09_crashpod (opus-4.6) 📄 31.0s 34.3s ±0% 32.5s ±0%
101_loki_historical_logs_pod_deleted (opus-4.6) 📄 85.3s 63.8s ↑34% 51.7s ↑65%
112_find_pvcs_by_uuid (opus-4.6) 📄 21.2s 19.7s ±0% 18.1s ↑17%
12_job_crashing (opus-4.6) 📄 45.8s 42.2s ±0% 42.4s ±0%
176_network_policy_blocking_traffic_no_skills (opus-4.6) 📄 58.1s 42.3s ↑37% 35.7s ↑63%
227_count_configmaps_per_namespace[0] (opus-4.6) 📄 20.7s 18.6s ↑12% 19.7s ±0%
243_pod_names_contain_service (opus-4.6) 📄 36.0s 38.3s ±0% 27.4s ↑31%
24_misconfigured_pvc (opus-4.6) 📄 40.9s 40.5s ±0% 35.8s ↑14%
43_current_datetime_from_prompt (opus-4.6) 📄 4.0s 3.1s ↑29% — —
51_logs_summarize_errors (opus-4.6) 📄 22.1s 21.4s ±0% 23.1s ±0%
61_exact_match_counting (opus-4.6) 📄 12.0s 9.3s ↑30% 10.8s ↑11%
Total (all, n=11) 34.3s 30.3s — 29.7s —
Comparable (m=11, b=10) 34.3s 30.3s ↑13% 29.7s ↑26%

Cost comparison:

Test case This branch master (8h ago) Δ vs master benchmark (14d ago) Δ vs benchmark
09_crashpod (opus-4.6) 📄 $0.2453 $0.2729 ↓10% $0.2616 ±0%
101_loki_historical_logs_pod_deleted (opus-4.6) 📄 $0.4582 $0.3876 ↑18% $0.3371 ↑36%
112_find_pvcs_by_uuid (opus-4.6) 📄 $0.2066 $0.2086 ±0% $0.2014 ±0%
12_job_crashing (opus-4.6) 📄 $0.3287 $0.3169 ±0% $0.3076 ±0%
176_network_policy_blocking_traffic_no_skills (opus-4.6) 📄 $0.3826 $0.3282 ↑17% $0.2914 ↑31%
227_count_configmaps_per_namespace[0] (opus-4.6) 📄 $0.2083 $0.2069 ±0% $0.2059 ±0%
243_pod_names_contain_service (opus-4.6) 📄 $0.2602 $0.2881 ±0% $0.2280 ↑14%
24_misconfigured_pvc (opus-4.6) 📄 $0.2869 $0.3109 ±0% $0.2831 ±0%
43_current_datetime_from_prompt (opus-4.6) 📄 $0.1176 $0.0120 ↑880% — —
51_logs_summarize_errors (opus-4.6) 📄 $0.2077 $0.2070 ±0% $0.2072 ±0%
61_exact_match_counting (opus-4.6) 📄 $0.1510 $0.1510 ±0% $0.1522 ±0%
Total (all, n=11) $0.2594 $0.2446 — $0.2475 —
Comparable (m=11, b=10) $0.2594 $0.2446 ±0% $0.2475 ↑11%

Total tokens comparison:

Test case This branch master (8h ago) Δ vs master benchmark (14d ago) Δ vs benchmark
09_crashpod (opus-4.6) 📄 82,285 104,084 ↓21% 103,497 ↓20%
101_loki_historical_logs_pod_deleted (opus-4.6) 📄 202,574 167,778 ↑21% 138,670 ↑46%
112_find_pvcs_by_uuid (opus-4.6) 📄 60,962 61,070 ±0% 61,169 ±0%
12_job_crashing (opus-4.6) 📄 159,498 135,709 ↑18% 133,893 ↑19%
176_network_policy_blocking_traffic_no_skills (opus-4.6) 📄 168,506 113,278 ↑49% 111,145 ↑52%
227_count_configmaps_per_namespace[0] (opus-4.6) 📄 76,261 76,279 ±0% 76,945 ±0%
243_pod_names_contain_service (opus-4.6) 📄 102,502 124,965 ↓18% 79,525 ↑29%
24_misconfigured_pvc (opus-4.6) 📄 107,438 131,123 ↓18% 108,047 ±0%
43_current_datetime_from_prompt (opus-4.6) 📄 16,876 16,898 ±0% — —
51_logs_summarize_errors (opus-4.6) 📄 77,659 77,246 ±0% 77,707 ±0%
61_exact_match_counting (opus-4.6) 📄 52,415 52,428 ±0% 52,942 ±0%
Total (all, n=11) 100,634 96,442 — 94,354 —
Comparable (m=11, b=10) 100,634 96,442 ±0% 94,354 ↑16%

Cached tokens comparison:

Test case This branch master (8h ago) Δ vs master benchmark (14d ago) Δ vs benchmark
09_crashpod (opus-4.6) 📄 56,697 77,473 ↓27% 77,391 ↓27%
101_loki_historical_logs_pod_deleted (opus-4.6) 📄 165,018 134,689 ↑23% 106,565 ↑55%
112_find_pvcs_by_uuid (opus-4.6) 📄 37,651 37,651 ±0% 38,002 ±0%
12_job_crashing (opus-4.6) 📄 130,171 106,020 ↑23% 104,761 ↑24%
176_network_policy_blocking_traffic_no_skills (opus-4.6) 📄 134,749 79,473 ↑70% 81,519 ↑65%
227_count_configmaps_per_namespace[0] (opus-4.6) 📄 53,360 53,666 ±0% 54,503 ±0%
243_pod_names_contain_service (opus-4.6) 📄 77,148 98,220 ↓21% 55,513 ↑39%
24_misconfigured_pvc (opus-4.6) 📄 79,548 102,442 ↓22% 80,270 ±0%
43_current_datetime_from_prompt (opus-4.6) 📄 — 16,795 — — —
51_logs_summarize_errors (opus-4.6) 📄 55,264 55,037 ±0% 55,443 ±0%
61_exact_match_counting (opus-4.6) 📄 34,279 34,285 ±0% 34,632 ±0%
Total (all, n=11) 74,899 72,341 — 68,860 —
Comparable (m=10, b=10) 82,388 77,896 ±0% 68,860 ↑20%

Turns comparison:

Test case This branch master (8h ago) Δ vs master benchmark (14d ago) Δ vs benchmark
09_crashpod (opus-4.6) 📄 4 5 ↓20% — —
101_loki_historical_logs_pod_deleted (opus-4.6) 📄 8 7 ↑14% — —
112_find_pvcs_by_uuid (opus-4.6) 📄 3 3 ±0% — —
12_job_crashing (opus-4.6) 📄 7 6 ↑17% — —
176_network_policy_blocking_traffic_no_skills (opus-4.6) 📄 7 5 ↑40% — —
227_count_configmaps_per_namespace[0] (opus-4.6) 📄 4 4 ±0% — —
243_pod_names_contain_service (opus-4.6) 📄 5 6 ↓17% — —
24_misconfigured_pvc (opus-4.6) 📄 5 6 ↓17% — —
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.6 4.5 — — —
Comparable (m=11, b=0) 4.6 4.5 ±0% — —

Tool calls comparison:

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

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/fix-eval-perf-task-mgmt-position -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 claude/fix-eval-perf-task-mgmt-position -f markers=regression -f filter=

@github-actions

github-actions Bot commented May 17, 2026 •

Copy link
Copy Markdown
Contributor

✅ Docker images ready for 8169c6e4 (built in 6m 36s)

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

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

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

@netlify

netlify Bot commented May 17, 2026

Copy link
Copy Markdown

✅ Deploy Preview for holmes-docs ready!

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

This branch has not been deployed

No deployments
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