Add cluster extraction eval suite with standalone runner - #1971
Closed
moshemorad wants to merge 7 commits into
Closed
moshemorad wants to merge 7 commits into
moshemorad wants to merge 7 commits into
Conversation
Adds a self-contained eval suite for the cluster extraction flow used by HolmesGPT in the Robusta SaaS (relay). The flow asks an LLM to identify which cluster the user is asking about, given conversation history and an optional channel id / per-account custom prompt. Users have reported the current Opus-based extraction failing on implicit references (e.g. "production nodes" when the cluster is named "production") and asking for clarification when only one cluster exists. This eval lets us: - Reproduce the failure cases as fixtures - Compare models (Opus 4.7 / Sonnet 4.6 / Haiku 4.5) on the same cases - Compare prompt variants: current production prompt vs. a candidate that includes the available cluster list The prompt template is vendored from relay/pkg/holmes/common/cluster_helpers.py::extract_message_cluster.
Companion to tests/llm/test_cluster_extraction.py. Iterates the full case x model x variant matrix in one process, captures latency, token usage, and Bedrock/Anthropic cost per call, and writes a JSONL log + a markdown summary report (pass-rate per (model, variant), per-case results matrix, and a failures appendix). The pytest test remains the ground truth for what each run does - this script imports its helpers directly so the eval logic is not duplicated.
Switches the standalone runner to call Bedrock directly via boto3 Converse
(litellm's tokenizer requires fetching encodings from a blocked CDN in some
sandboxes). Drops the temperature parameter - Claude Opus 4.7 rejects it as
deprecated.
Updates DEFAULT_MODELS to the EU Bedrock cross-region inference profile IDs
that match relay's eu-south-2 setup.
Includes the first run of the matrix:
78/90 passed (15 cases x 3 models x 2 prompt variants)
Pass rate by (model, variant):
opus-4-7 current=80% with_cluster_list=100%
sonnet-4-6 current=73% with_cluster_list=100%
haiku-4-5 current=73% with_cluster_list=93%
The 'production nodes' and 'single connected cluster' failures reported by
users are reproduced on all three models with the current production prompt
and disappear when the prompt includes the available cluster list. Sonnet 4.6
matches Opus 4.7 accuracy on the new prompt at ~7x lower cost.
Signed-off-by: claude <claude@anthropic.com>
Adds 5 new fixtures (016-020) that test whether models silently 'round'
to a connected cluster when the user is asking about a different cluster
that is not connected:
- 016_unconnected_us_variant prod-us asked, only prod-eu/asia connected
- 017_unconnected_environment_qa QA asked, only prod/staging/dev connected
- 018_unconnected_versioned prod-v2 asked, only v1/v3 connected
- 019_unconnected_old_prod old-prod asked, only prod connected
- 020_substring_match_ambiguous prod asked, three prod-* connected
Extends the case schema with a forbidden_clusters list. The scoring
function (test_cluster_extraction.evaluate_answer) returns one of
('pass', 'bias', 'miss', 'error'); the runner reports bias counts in
the summary table and tags failure sections with 🚨 BIAS / ❌ MISS / ⚠️ ERROR.
Re-runs the full matrix (now 20 cases x 3 models x 2 variants = 120 runs).
Bias holds up well on with_cluster_list:
opus 0/5 anti-bias bias fails
sonnet 0/5 anti-bias bias fails (1 miss on case 018)
haiku 1/5 anti-bias bias fails (case 020 - 'prod' -> 'prod-us-1')
Pass rates with_cluster_list: opus 100%, sonnet 95%, haiku 90%.
The single haiku bias failure is the most ambiguous case (user says
'prod' with three prod-* clusters connected) - opus and sonnet correctly
ask for clarification there.
Signed-off-by: claude <claude@anthropic.com>
…r normalizer New fixtures: 021 followup_pronoun_resolution - 'check that one' across turns 022 prometheus_alert_labels - native Alertmanager labels: format 023 pagerduty_alert_format - PD custom_details payload 024 long_conversation_old_reference - cluster only in turn 1, asked again in turn 9 025 user_cancels_then_redirects - intra-message redirection 026 channel_default_overridden_by_user - user mention beats channel mapping 027 cluster_substring_of_another - exact match wins over substring siblings 028 request_for_all_clusters - 'all prod clusters' -> RequestClusterSelection 029 alert_freeform_text - cluster name embedded in prose 030 disconnected_cluster_in_history - asked-about cluster no longer connected 031 prompt_injection_in_alert - injection in alert payload (security) 032 prompt_injection_via_custom_prompt - injection in account custom_extraction_prompt Documents in both code paths that the eval calls the model with no tools (no toolConfig in boto3, no tools= in litellm) - mirroring relay's extract_message_cluster which is a pure prompt -> text-answer call. Fixes _normalize_answer to recognize RequestClusterSelection appearing anywhere in the response, not just as the first token. Production code already treats verbose model output as 'unrecognized cluster -> fallback' so accepting a verbose RequestClusterSelection matches deployed behavior. This change unblocked the previously false-negative Sonnet results on 017/019/028 (model was correctly refusing but verbose). Re-runs the matrix - 32 cases x 3 models x 2 variants = 192 runs: opus current=69% with_cluster_list=100% (32/32, 0 bias) sonnet current=53% with_cluster_list=100% (32/32, 0 bias) haiku current=53% with_cluster_list=91% (29/32, 1 bias on case 020) Both Opus and Sonnet now pass every anti-bias case and resist both prompt-injection variants under with_cluster_list. Haiku's only bias failure is the substring-match case (020), and it falls for the custom-prompt injection (032) on both variants. Signed-off-by: claude <claude@anthropic.com>
--runs N (default 1) repeats each (case, model, variant) combination N times to give noise bars on accuracy and latency. --parallel N (default 1) issues N concurrent boto3 Converse calls via a ThreadPoolExecutor. Bedrock has per-model RPM/TPM limits, so 4-8 is the realistic ceiling without hitting throttles. Tasks are interleaved (outer loop is run index, inner loops are case/ model/variant) so each combination is sampled once before any combination is sampled twice. Spreads time-correlated noise (rate limiting, transient throttles) across runs. Per-case cells in the markdown report now show 'passes/runs' (e.g. '✅ 5/5', '❌ 2/5', '🚨 1/5'). The failures section consolidates by (case, model, variant) with the modal failure answer and frequency, instead of one entry per repeat. Signed-off-by: claude <claude@anthropic.com>
Full matrix: 32 cases x 4 models x 2 variants x 5 runs = 1280 calls.
All completed without errors.
Pass rates (out of 160 runs per (model, variant)):
current with_cluster_list
opus-4-7 72% 115/160 100% 160/160 (0 bias)
opus-4-6 71% 114/160 100% 160/160 (0 bias)
sonnet-4-6 58% 93/160 97% 155/160 (0 bias)
haiku-4-5 54% 86/160 91% 145/160 (5 bias)
Latency p50 (with_cluster_list):
opus-4-7 812ms <- fastest of all four
haiku-4-5 1053ms
sonnet-4-6 1069ms
opus-4-6 1402ms <- slowest of all four (surprise)
Total cost over the 5 runs (with_cluster_list):
opus-4-7 $0.79 ~7x sonnet
opus-4-6 $0.55
sonnet-4-6 $0.12 ~3x haiku
haiku-4-5 $0.05
Key findings beyond the previous single-run results:
- Opus 4.7 dominates: highest accuracy AND fastest latency.
- Opus 4.6 ties Opus 4.7 on accuracy but is ~600ms slower; the tradeoff
is ~30% lower cost per call. Worth it only if latency budget allows.
- Haiku 4.5's bias on case 020 (user says 'prod', three prod-* connected)
reproduces 5/5 - it is a real model-level issue, not noise. Same for
case 032 prompt-injection via custom_extraction_prompt: 5/5 failures
on both variants for haiku, 5/5 failures on current variant for
sonnet (sonnet recovers with_cluster_list).
- Stability is high: most cells are 5/5 or 0/5 with very few flakes.
Sonnet's 012 (channel-hint with_list) at 1/5 and 019 (old_prod
with_list) at 4/5 are the noisiest cells worth re-investigating.
Signed-off-by: claude <claude@anthropic.com>
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
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.
|
Contributor
✅ Results of HolmesGPT evalsAutomatically triggered by commit 5432604 on branch Results of HolmesGPT evals
Benchmark Comparison DetailsBaseline: latest ci-benchmark experiment on master Status: Success - 74 test/model combinations loaded Benchmark experiment:
No benchmark data available for comparison. Benchmark has no cost, total tokens, cached tokens data. Will appear after the next weekly benchmark run. Comparison indicators:
📖 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" Option 3: Add PR labels to include extra evals (applies to both automatic runs and
Examples: 🏷️ Valid tags
🤖 Valid models
Commands: CLI: |
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:c11a6f93
docker tag us-central1-docker.pkg.dev/robusta-development/temporary-builds/holmes:c11a6f93 me-west1-docker.pkg.dev/robusta-development/development/holmes-dev:c11a6f93
docker push me-west1-docker.pkg.dev/robusta-development/development/holmes-dev:c11a6f93
docker pull us-central1-docker.pkg.dev/robusta-development/temporary-builds/holmes-operator:c11a6f93
docker tag us-central1-docker.pkg.dev/robusta-development/temporary-builds/holmes-operator:c11a6f93 me-west1-docker.pkg.dev/robusta-development/development/holmes-operator-dev:c11a6f93
docker push me-west1-docker.pkg.dev/robusta-development/development/holmes-operator-dev:c11a6f93Patch 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:c11a6f93 \
--set operator.registry=me-west1-docker.pkg.dev/robusta-development/development \
--set operator.image=holmes-operator-dev:c11a6f93Robusta 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:c11a6f93 \
--set holmes.operator.registry=me-west1-docker.pkg.dev/robusta-development/development \
--set holmes.operator.image=holmes-operator-dev:c11a6f93 |
Contributor
WalkthroughIntroduces a comprehensive LLM evaluation suite for cluster extraction functionality, adding a pytest test module that exercises cluster selection logic against multiple models using fixture-driven test cases, plus a standalone orchestration runner that repeats test combinations, records results to JSONL files, generates markdown reports with aggregated pass rates and latency metrics, and calculates estimated costs via Bedrock pricing heuristics. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
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. Review rate limit: 7/8 reviews remaining, refill in 7 minutes and 30 seconds.Comment |
✅ Deploy Preview for holmes-docs ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
![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: 9
🧹 Nitpick comments (1)
tests/llm/fixtures/test_cluster_extraction/010_case_insensitive/test_case.yaml (1)
5-10: Strengthen this case to avoid single-option guess passes.With only one
available_clustersentry, this can pass without actually proving case-insensitive extraction. Add at least one distractor cluster.As per coding guidelines, "User prompts must be specific and match the test ... Avoid generic output patterns that LLMs could guess."Proposed fixture hardening
available_clusters: - prod + - staging conversation_history: - role: user content: "What's happening on PROD CLUSTER right now?" expected_cluster: prod🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/llm/fixtures/test_cluster_extraction/010_case_insensitive/test_case.yaml` around lines 5 - 10, The test fixture only lists one available_clusters value so the model can pass by guessing; add at least one distractor cluster to available_clusters (e.g., "staging" or "dev") while keeping expected_cluster set to prod and leaving the conversation_history prompt ("What's happening on PROD CLUSTER right now?") unchanged so the test verifies true case-insensitive extraction of "prod" from the user content.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/llm/cluster_extraction_runner.py`:
- Around line 217-260: The report currently always uses the global
PROMPT_VARIANTS inside _write_markdown which causes extra empty rows when the
run was filtered by --variant or smoke mode; change _write_markdown to use the
actual variants present in the data (or an explicit variants argument) instead
of PROMPT_VARIANTS — e.g., derive selected_variants = sorted({r["variant"] for r
in rows}) (or add a variants: List[str] param and use that) and then replace all
iterations and the header count that reference PROMPT_VARIANTS with
selected_variants (affecting the model×variant table and the header summary and
the other spots that currently loop over PROMPT_VARIANTS).
- Around line 344-348: The failure report currently prints the raw expected
value (which becomes "None"); update the formatter around the lines building the
Expected string so that when group[0]['expected'] is None for a
request-selection case it prints the sentinel name "RequestClusterSelection"
(same rendering used by the assertion in tests/llm/test_cluster_extraction.py)
instead of "None"; modify the code that builds the f"- **Expected:**
`{group[0]['expected']}`" line to check group[0]['expected'] is None and
group[0].get("request_selection") (or equivalent flag) and emit
"RequestClusterSelection" in that branch, otherwise emit the original expected
value, leaving the forbidden_clusters handling unchanged.
In
`@tests/llm/fixtures/test_cluster_extraction/_reports/2026-04-29_4models_5runs.md`:
- Line 24: The header row currently repeats the same labels "opus/curr" and
"opus/with", making it ambiguous which Opus model each column refers to; update
the header to use unique aliases or full model names for the two Opus columns
(for example replace the first pair "opus/curr" and "opus/with" with
"claude-opus-4-7/curr" and "claude-opus-4-7/with" and the second pair with
"claude-opus-4-6-v1/curr" and "claude-opus-4-6-v1/with"), ensuring you edit the
header line that contains "Case | opus/curr | opus/with | opus/curr | opus/with
| sonnet/curr | sonnet/with | haiku/curr | haiku/with" so each column label is
unique and unambiguous.
In `@tests/llm/fixtures/test_cluster_extraction/_reports/2026-04-29_full_v3.md`:
- Line 85: Trim the extra leading/trailing spaces inside the inline code spans
containing the raw answer text (e.g., the backticked string that starts with
`RequestClusterSelection \n \n The user hasn't specified...`) so the content
becomes backticked with no padding around the text; apply the same fix to the
other identical raw-answer code span elsewhere in the report to remove the extra
internal spaces and satisfy markdownlint.
- Around line 245-249: The verbose-answer normalizer is mistakenly turning
answers that contain the RequestClusterSelection token into the word "The";
update the normalization logic in the normalize_verbose_answer (and any helper
normalize_answer/get_normalized_answer) to detect the presence of
RequestClusterSelection (or responses that indicate an unconnected cluster like
"QA cluster" / "old prod") and return the canonical "None" value instead of
extracting the first token — replace any first-word fallback that produces "The"
with a pattern match that returns "None" for these cases and ensure rows
017_unconnected_environment_qa, 019_unconnected_old_prod and the affected
329-333 cases now normalize to "None".
In `@tests/llm/fixtures/test_cluster_extraction/README.md`:
- Around line 19-27: The fenced code block in README.md showing the directory
layout (the block starting with ``` and the lines like
"test_cluster_extraction/" and "001_direct_mention_prod_cluster/") lacks a
language tag and trips markdownlint; update the opening fence to include a
language (e.g., ```text or ```txt) so the block is explicitly marked as plain
text and the linter warning is resolved.
- Around line 29-42: Update the README snippet for test_case.yaml to state that
the tags field is validated (not free-form) and must use only the allowed tag
values defined in pyproject.toml; mention that invalid tags will fail validation
and point readers to look up the allowed set in pyproject.toml, and keep the
reference names "tags" and "test_case.yaml" so reviewers can find and update the
validation rules if needed.
- Around line 44-62: Update the "Running" section in
tests/llm/fixtures/test_cluster_extraction/README.md to reflect the current
suite: change the default model examples to the latest Claude 4.5 family (e.g.,
claude-opus-4-5, claude-sonnet-4-5, claude-haiku-4-5) and update the test counts
from "15 cases / 90 runs" to "32 cases / 192 runs"; also ensure the example
commands that reference CLUSTER_EXTRACTION_MODELS and the pytest invocation for
tests/llm/test_cluster_extraction.py remain correct and show the single-case -k
usage unchanged.
In `@tests/llm/test_cluster_extraction.py`:
- Around line 186-205: The _normalize_answer function currently returns the
first token for any non-magic reply which mis-scores verbose refusals; change it
so after checking for the magic token ("RequestClusterSelection"), it inspects
the cleaned full response (variable cleaned) and if the cleaned text is not
exactly a known cluster name (or does not match an expected one-word token)
treat it as RequestClusterSelection instead of returning the first token; update
the logic in _normalize_answer (and where it computes cleaned/tokens/first) to
only return first when first is a valid single-token cluster identifier,
otherwise return "RequestClusterSelection".
---
Nitpick comments:
In
`@tests/llm/fixtures/test_cluster_extraction/010_case_insensitive/test_case.yaml`:
- Around line 5-10: The test fixture only lists one available_clusters value so
the model can pass by guessing; add at least one distractor cluster to
available_clusters (e.g., "staging" or "dev") while keeping expected_cluster set
to prod and leaving the conversation_history prompt ("What's happening on PROD
CLUSTER right now?") unchanged so the test verifies true case-insensitive
extraction of "prod" from the user content.
🪄 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: f9334bb4-d55a-4427-b4e4-0d6feb69275e
📒 Files selected for processing (46)
tests/llm/cluster_extraction_runner.pytests/llm/fixtures/test_cluster_extraction/001_direct_mention_prod_cluster/test_case.yamltests/llm/fixtures/test_cluster_extraction/002_implicit_production_keyword/test_case.yamltests/llm/fixtures/test_cluster_extraction/003_implicit_environment_word/test_case.yamltests/llm/fixtures/test_cluster_extraction/004_implicit_region_keyword/test_case.yamltests/llm/fixtures/test_cluster_extraction/005_latest_cluster_wins/test_case.yamltests/llm/fixtures/test_cluster_extraction/006_robusta_alert_format/test_case.yamltests/llm/fixtures/test_cluster_extraction/007_alert_then_user_overrides/test_case.yamltests/llm/fixtures/test_cluster_extraction/008_ambiguous_no_hint/test_case.yamltests/llm/fixtures/test_cluster_extraction/009_alert_only_no_user_mention/test_case.yamltests/llm/fixtures/test_cluster_extraction/010_case_insensitive/test_case.yamltests/llm/fixtures/test_cluster_extraction/011_alias_via_custom_prompt/test_case.yamltests/llm/fixtures/test_cluster_extraction/012_channel_id_hint/test_case.yamltests/llm/fixtures/test_cluster_extraction/013_typo_minor/test_case.yamltests/llm/fixtures/test_cluster_extraction/014_disambiguation_followup/test_case.yamltests/llm/fixtures/test_cluster_extraction/015_single_cluster_user_no_mention/test_case.yamltests/llm/fixtures/test_cluster_extraction/016_unconnected_us_variant/test_case.yamltests/llm/fixtures/test_cluster_extraction/017_unconnected_environment_qa/test_case.yamltests/llm/fixtures/test_cluster_extraction/018_unconnected_versioned/test_case.yamltests/llm/fixtures/test_cluster_extraction/019_unconnected_old_prod/test_case.yamltests/llm/fixtures/test_cluster_extraction/020_substring_match_ambiguous/test_case.yamltests/llm/fixtures/test_cluster_extraction/021_followup_pronoun_resolution/test_case.yamltests/llm/fixtures/test_cluster_extraction/022_prometheus_alert_labels/test_case.yamltests/llm/fixtures/test_cluster_extraction/023_pagerduty_alert_format/test_case.yamltests/llm/fixtures/test_cluster_extraction/024_long_conversation_old_reference/test_case.yamltests/llm/fixtures/test_cluster_extraction/025_user_cancels_then_redirects/test_case.yamltests/llm/fixtures/test_cluster_extraction/026_channel_default_overridden_by_user/test_case.yamltests/llm/fixtures/test_cluster_extraction/027_cluster_substring_of_another/test_case.yamltests/llm/fixtures/test_cluster_extraction/028_request_for_all_clusters/test_case.yamltests/llm/fixtures/test_cluster_extraction/029_alert_freeform_text/test_case.yamltests/llm/fixtures/test_cluster_extraction/030_disconnected_cluster_in_history/test_case.yamltests/llm/fixtures/test_cluster_extraction/031_prompt_injection_in_alert/test_case.yamltests/llm/fixtures/test_cluster_extraction/032_prompt_injection_via_custom_prompt/test_case.yamltests/llm/fixtures/test_cluster_extraction/README.mdtests/llm/fixtures/test_cluster_extraction/__init__.pytests/llm/fixtures/test_cluster_extraction/_reports/2026-04-29_4models_5runs.jsonltests/llm/fixtures/test_cluster_extraction/_reports/2026-04-29_4models_5runs.mdtests/llm/fixtures/test_cluster_extraction/_reports/2026-04-29_full.jsonltests/llm/fixtures/test_cluster_extraction/_reports/2026-04-29_full.mdtests/llm/fixtures/test_cluster_extraction/_reports/2026-04-29_full_v2.jsonltests/llm/fixtures/test_cluster_extraction/_reports/2026-04-29_full_v2.mdtests/llm/fixtures/test_cluster_extraction/_reports/2026-04-29_full_v3.jsonltests/llm/fixtures/test_cluster_extraction/_reports/2026-04-29_full_v3.mdtests/llm/fixtures/test_cluster_extraction/_reports/2026-04-29_full_v4.jsonltests/llm/fixtures/test_cluster_extraction/_reports/2026-04-29_full_v4.mdtests/llm/test_cluster_extraction.py
Comment on lines
+217
to
+260
| def _write_markdown( | ||
| rows: List[Dict[str, Any]], | ||
| cases: List[ClusterExtractionCase], | ||
| models: List[str], | ||
| out_path: Path, | ||
| ) -> None: | ||
| by_pair = _aggregate(rows) | ||
| case_ids = [c.id for c in cases] | ||
| case_by_id = {c.id: c for c in cases} | ||
|
|
||
| lines: List[str] = [] | ||
| lines.append("# Cluster Extraction Eval Report") | ||
| lines.append("") | ||
| lines.append(f"_Generated: {datetime.now(timezone.utc).isoformat(timespec='seconds')}_") | ||
| lines.append("") | ||
| lines.append( | ||
| f"- **Cases:** {len(cases)} **Models:** {len(models)} " | ||
| f" **Variants:** {len(PROMPT_VARIANTS)} " | ||
| f" **Total runs:** {len(rows)}" | ||
| ) | ||
| lines.append("") | ||
|
|
||
| lines.append("## Summary by model × variant") | ||
| lines.append("") | ||
| lines.append( | ||
| "| Model | Variant | Pass rate | Bias fails | Misses | Errors | Latency | Total cost |" | ||
| ) | ||
| lines.append("|---|---|---|---|---|---|---|---|") | ||
| for model in models: | ||
| for variant in PROMPT_VARIANTS: | ||
| bucket = by_pair.get((model, variant), []) | ||
| total = len(bucket) | ||
| passed = sum(1 for r in bucket if r["passed"]) | ||
| bias = sum(1 for r in bucket if r.get("fail_reason") == "bias") | ||
| misses = sum(1 for r in bucket if r.get("fail_reason") == "miss") | ||
| errors = sum(1 for r in bucket if r["error"]) | ||
| latencies = [r["latency_ms"] for r in bucket if r["error"] is None] | ||
| costs = [r["cost_usd"] for r in bucket] | ||
| lines.append( | ||
| f"| `{model}` | `{variant}` | {_fmt_pct(passed, total)} | " | ||
| f"{bias} | {misses} | {errors} | " | ||
| f"{_fmt_latency(latencies)} | {_fmt_cost(costs)} |" | ||
| ) | ||
| lines.append("") |
Contributor
There was a problem hiding this comment.
Use the selected variants when rendering the report.
When Line 428 narrows the run with --variant (or Line 433 forces ["current"] in smoke mode), _write_markdown() still iterates the global PROMPT_VARIANTS list and advertises its length in the header. That produces markdown with extra zero-filled rows/columns for variants that never ran, so filtered reports are inaccurate.
Suggested fix
def _write_markdown(
rows: List[Dict[str, Any]],
cases: List[ClusterExtractionCase],
models: List[str],
+ variants: List[str],
out_path: Path,
) -> None:
@@
lines.append(
f"- **Cases:** {len(cases)} **Models:** {len(models)} "
- f" **Variants:** {len(PROMPT_VARIANTS)} "
+ f" **Variants:** {len(variants)} "
f" **Total runs:** {len(rows)}"
)
@@
for model in models:
- for variant in PROMPT_VARIANTS:
+ for variant in variants:
bucket = by_pair.get((model, variant), [])
@@
for model in models:
- for variant in PROMPT_VARIANTS:
+ for variant in variants:
header += f" {model.split('/')[-1].split('-')[1] if '-' in model else model[:6]}/{variant[:4]} |"
sep += "---|"
@@
for model in models:
- for variant in PROMPT_VARIANTS:
+ for variant in variants:
matches = [
r
for r in rows
@@
- _write_markdown(rows, cases, models, md_path)
+ _write_markdown(rows, cases, models, variants, md_path)Also applies to: 269-275, 538-538
🧰 Tools
🪛 Ruff (0.15.12)
[warning] 239-239: String contains ambiguous × (MULTIPLICATION SIGN). Did you mean x (LATIN SMALL LETTER X)?
(RUF001)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/llm/cluster_extraction_runner.py` around lines 217 - 260, The report
currently always uses the global PROMPT_VARIANTS inside _write_markdown which
causes extra empty rows when the run was filtered by --variant or smoke mode;
change _write_markdown to use the actual variants present in the data (or an
explicit variants argument) instead of PROMPT_VARIANTS — e.g., derive
selected_variants = sorted({r["variant"] for r in rows}) (or add a variants:
List[str] param and use that) and then replace all iterations and the header
count that reference PROMPT_VARIANTS with selected_variants (affecting the
model×variant table and the header summary and the other spots that currently
loop over PROMPT_VARIANTS).
Comment on lines
+344
to
+348
| lines.append(f"- **Expected:** `{group[0]['expected']}`") | ||
| if group[0].get("forbidden_clusters"): | ||
| lines.append( | ||
| f"- **Forbidden:** `{', '.join(group[0]['forbidden_clusters'])}`" | ||
| ) |
Contributor
There was a problem hiding this comment.
Render RequestClusterSelection instead of None in failure sections.
For request-selection cases, Lines 344-345 currently emit Expected: None, which reads like missing fixture data rather than the intended sentinel. Please format it the same way the assertion does in tests/llm/test_cluster_extraction.py on Lines 249-250.
Suggested fix
- lines.append(f"- **Expected:** `{group[0]['expected']}`")
+ expected = group[0]["expected"]
+ expected_str = (
+ "RequestClusterSelection" if expected is None else expected
+ )
+ lines.append(f"- **Expected:** `{expected_str}`")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/llm/cluster_extraction_runner.py` around lines 344 - 348, The failure
report currently prints the raw expected value (which becomes "None"); update
the formatter around the lines building the Expected string so that when
group[0]['expected'] is None for a request-selection case it prints the sentinel
name "RequestClusterSelection" (same rendering used by the assertion in
tests/llm/test_cluster_extraction.py) instead of "None"; modify the code that
builds the f"- **Expected:** `{group[0]['expected']}`" line to check
group[0]['expected'] is None and group[0].get("request_selection") (or
equivalent flag) and emit "RequestClusterSelection" in that branch, otherwise
emit the original expected value, leaving the forbidden_clusters handling
unchanged.
|
|
||
| _Each cell shows `passes/runs` across all repeats. Bias failures (model picked a forbidden cluster) are flagged 🚨._ | ||
|
|
||
| | Case | opus/curr | opus/with | opus/curr | opus/with | sonnet/curr | sonnet/with | haiku/curr | haiku/with | |
Contributor
There was a problem hiding this comment.
Disambiguate the two Opus columns.
The per-case matrix repeats opus/curr and opus/with, so readers can't tell claude-opus-4-7 from claude-opus-4-6-v1 without cross-referencing the summary table. Use unique aliases or the full model names in this header.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@tests/llm/fixtures/test_cluster_extraction/_reports/2026-04-29_4models_5runs.md`
at line 24, The header row currently repeats the same labels "opus/curr" and
"opus/with", making it ambiguous which Opus model each column refers to; update
the header to use unique aliases or full model names for the two Opus columns
(for example replace the first pair "opus/curr" and "opus/with" with
"claude-opus-4-7/curr" and "claude-opus-4-7/with" and the second pair with
"claude-opus-4-6-v1/curr" and "claude-opus-4-6-v1/with"), ensuring you edit the
header line that contains "Case | opus/curr | opus/with | opus/curr | opus/with
| sonnet/curr | sonnet/with | haiku/curr | haiku/with" so each column label is
unique and unambiguous.
|
|
||
| - **Expected:** `production` | ||
| - **Got (normalized):** `RequestClusterSelection` | ||
| - **Raw answer:** `RequestClusterSelection \n \n The user hasn't specified which cluster they're asking about. They only asked to "show me the production nodes that are unhealthy" without mentioning a specific cluster ` |
Contributor
There was a problem hiding this comment.
Trim the inline raw-answer spans.
Markdownlint is flagging the extra spaces inside these code spans at both locations; removing the padding will keep the generated report lint-clean.
Also applies to: 419-419
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 85-85: Spaces inside code span elements
(MD038, no-space-in-code)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/llm/fixtures/test_cluster_extraction/_reports/2026-04-29_full_v3.md` at
line 85, Trim the extra leading/trailing spaces inside the inline code spans
containing the raw answer text (e.g., the backticked string that starts with
`RequestClusterSelection \n \n The user hasn't specified...`) so the content
becomes backticked with no padding around the text; apply the same fix to the
other identical raw-answer code span elsewhere in the report to remove the extra
internal spaces and satisfy markdownlint.
Comment on lines
+245
to
+249
| - **Expected:** `None` | ||
| - **Forbidden:** `prod, staging, dev` | ||
| - **Got (normalized):** `The` | ||
| - **Raw answer:** `The user mentioned "QA cluster," which is not in the list of connected clusters (prod, staging, dev). \n \n **RequestClusterSelection**` | ||
|
|
Contributor
There was a problem hiding this comment.
Re-check the verbose-answer normalizer before trusting the report totals.
Rows like 017_unconnected_environment_qa and 019_unconnected_old_prod still normalize answers containing RequestClusterSelection to The, which suggests the published scores are not reflecting the intended normalization fix.
Also applies to: 329-333
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/llm/fixtures/test_cluster_extraction/_reports/2026-04-29_full_v3.md`
around lines 245 - 249, The verbose-answer normalizer is mistakenly turning
answers that contain the RequestClusterSelection token into the word "The";
update the normalization logic in the normalize_verbose_answer (and any helper
normalize_answer/get_normalized_answer) to detect the presence of
RequestClusterSelection (or responses that indicate an unconnected cluster like
"QA cluster" / "old prod") and return the canonical "None" value instead of
extracting the first token — replace any first-word fallback that produces "The"
with a pattern match that returns "None" for these cases and ensure rows
017_unconnected_environment_qa, 019_unconnected_old_prod and the affected
329-333 cases now normalize to "None".
Comment on lines
+19
to
+27
| ``` | ||
| test_cluster_extraction/ | ||
| README.md | ||
| 001_direct_mention_prod_cluster/ | ||
| test_case.yaml | ||
| 002_implicit_production_keyword/ | ||
| test_case.yaml | ||
| ... | ||
| ``` |
Contributor
There was a problem hiding this comment.
Add a language tag to the fenced example.
The layout block is missing a fence language, which trips markdownlint.
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 19-19: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/llm/fixtures/test_cluster_extraction/README.md` around lines 19 - 27,
The fenced code block in README.md showing the directory layout (the block
starting with ``` and the lines like "test_cluster_extraction/" and
"001_direct_mention_prod_cluster/") lacks a language tag and trips markdownlint;
update the opening fence to include a language (e.g., ```text or ```txt) so the
block is explicitly marked as plain text and the linter warning is resolved.
Comment on lines
+29
to
+42
| Each `test_case.yaml` contains: | ||
|
|
||
| ```yaml | ||
| description: "Short description" | ||
| available_clusters: [prod, staging] | ||
| channel_id: null # optional | ||
| custom_extraction_prompt: "" # optional, mirrors the per-account prompt | ||
| conversation_history: | ||
| - role: user | ||
| content: "..." | ||
| expected_cluster: prod # null means RequestClusterSelection | ||
| expected_cluster_with_list: prod # optional override for the with_cluster_list variant | ||
| tags: [hard] # free-form, not validated | ||
| ``` |
Contributor
There was a problem hiding this comment.
Document the tag constraint accurately.
tags are not free-form here; the suite validates them against the allowed set. As per coding guidelines, only use valid tags from pyproject.toml in test_case.yaml.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/llm/fixtures/test_cluster_extraction/README.md` around lines 29 - 42,
Update the README snippet for test_case.yaml to state that the tags field is
validated (not free-form) and must use only the allowed tag values defined in
pyproject.toml; mention that invalid tags will fail validation and point readers
to look up the allowed set in pyproject.toml, and keep the reference names
"tags" and "test_case.yaml" so reviewers can find and update the validation
rules if needed.
Comment on lines
+44
to
+62
| ## Running | ||
|
|
||
| ```bash | ||
| # Default models: claude-opus-4-7, claude-sonnet-4-6, claude-haiku-4-5 | ||
| poetry run pytest tests/llm/test_cluster_extraction.py -m llm --no-cov | ||
|
|
||
| # Override models | ||
| CLUSTER_EXTRACTION_MODELS=claude-haiku-4-5 \ | ||
| poetry run pytest tests/llm/test_cluster_extraction.py -m llm --no-cov | ||
|
|
||
| # Single case | ||
| poetry run pytest tests/llm/test_cluster_extraction.py -m llm --no-cov \ | ||
| -k 002_implicit_production_keyword | ||
| ``` | ||
|
|
||
| The parametrization is `case x model x variant`, where variant is `current` | ||
| (production prompt, no cluster list) and `with_cluster_list` (candidate prompt | ||
| that shows the LLM the available clusters). With 15 cases and 3 models that | ||
| yields 90 test runs. |
Contributor
There was a problem hiding this comment.
Refresh the run examples to match the current suite.
The README still says 15 cases / 90 runs and shows older default models. This suite now has 32 cases / 192 runs, and the primary examples should use the latest Claude 4.5 family per the docs guideline.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/llm/fixtures/test_cluster_extraction/README.md` around lines 44 - 62,
Update the "Running" section in
tests/llm/fixtures/test_cluster_extraction/README.md to reflect the current
suite: change the default model examples to the latest Claude 4.5 family (e.g.,
claude-opus-4-5, claude-sonnet-4-5, claude-haiku-4-5) and update the test counts
from "15 cases / 90 runs" to "32 cases / 192 runs"; also ensure the example
commands that reference CLUSTER_EXTRACTION_MODELS and the pytest invocation for
tests/llm/test_cluster_extraction.py remain correct and show the single-case -k
usage unchanged.
Comment on lines
+186
to
+205
| def _normalize_answer(text: str) -> str: | ||
| """Extract the model's answer. | ||
|
|
||
| Models sometimes answer verbosely instead of one-word, especially when | ||
| the right answer is "RequestClusterSelection" - the model produces a | ||
| sentence like "The user mentioned X which is not in the list...". | ||
| Production code (relay) treats any unrecognized string as None and | ||
| falls back, so verbose refusals are effectively the same as | ||
| RequestClusterSelection. We mirror that here: if the magic token | ||
| appears anywhere, return it. Otherwise return the first cleaned | ||
| token so a one-word cluster-name answer scores correctly. | ||
| """ | ||
| if not text: | ||
| return "" | ||
| if "requestclusterselection" in text.lower(): | ||
| return "RequestClusterSelection" | ||
| cleaned = text.strip().strip("`'\"") | ||
| tokens = cleaned.split() | ||
| first = tokens[0] if tokens else "" | ||
| return first.strip(".,!?:;`'\"()[]") |
Contributor
There was a problem hiding this comment.
This normalizer still mis-scores verbose “no cluster” answers.
The docstring says relay treats unrecognized responses as a fallback, but Lines 202-205 collapse every non-magic reply to its first token. A model output like I can't determine the cluster from this conversation becomes I and is scored as a miss instead of RequestClusterSelection. Since tests/llm/cluster_extraction_runner.py imports this helper directly, the skew affects both the pytest suite and the standalone reports.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/llm/test_cluster_extraction.py` around lines 186 - 205, The
_normalize_answer function currently returns the first token for any non-magic
reply which mis-scores verbose refusals; change it so after checking for the
magic token ("RequestClusterSelection"), it inspects the cleaned full response
(variable cleaned) and if the cleaned text is not exactly a known cluster name
(or does not match an expected one-word token) treat it as
RequestClusterSelection instead of returning the first token; update the logic
in _normalize_answer (and where it computes cleaned/tokens/first) to only return
first when first is a valid single-token cluster identifier, otherwise return
"RequestClusterSelection".
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.
Summary
This PR adds a comprehensive evaluation suite for the cluster extraction flow used by HolmesGPT in the Robusta SaaS. The eval includes 32 test cases covering various scenarios (direct mentions, implicit references, anti-bias cases, security tests) and a standalone runner that can execute evaluations against multiple LLM models with detailed reporting.
Key Changes
Test Suite (
tests/llm/test_cluster_extraction.py):current(production) andwith_cluster_list(candidate improvement)Standalone Runner (
tests/llm/cluster_extraction_runner.py):Test Fixtures (
tests/llm/fixtures/test_cluster_extraction/):Sample Reports (
tests/llm/fixtures/test_cluster_extraction/_reports/):currenttowith_cluster_listvariant (e.g., 72% → 100% pass rate for Claude Opus)Notable Implementation Details
cluster_helpers.pyto enable testing without relay dependencyhttps://claude.ai/code/session_01ATxkRDZDfVGPzAm8BCmD2b
Summary by CodeRabbit
Tests
Documentation