feat: JudgeLLM evaluation with ProposalAmender#248
Open
rioloc wants to merge 1 commit into
Open
Conversation
Contributor
WalkthroughThis pull request introduces an LLM-judged evaluation framework for agentic remediation workflows. It extracts phase derivation into a shared utility, creates a Kubernetes CLI abstraction, implements a ChangesProposal Evaluation and Enrichment Pipeline
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
rioloc
force-pushed
the
feat/judge-evaluation
branch
2 times, most recently
from
354b221 to
5c2f4b1
Compare
|
|
||
| import json | ||
| import os | ||
| import subprocess |
rioloc
force-pushed
the
feat/judge-evaluation
branch
from
ed62f0f to
5492575
Compare
rioloc
force-pushed
the
feat/judge-evaluation
branch
from
5492575 to
c47a46f
Compare
Collaborator
Author
|
@CodeRabbit review |
Contributor
✅ Actions performedReview triggered.
|
![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: 3
🧹 Nitpick comments (2)
tests/integration/test_evaluation_data_proposal.yaml (1)
95-104: 💤 Low value
expected_responseis likely unused for this metric.
custom:proposal_evaluation_correctnessis a turn-level LLM-as-judge metric that requires onlyresponse(no ground truth), soexpected_responsehere will not be consulted during scoring. It's harmless but can mislead readers into thinking the judge compares against it. Consider dropping it or adding a brief comment clarifying it's documentation-only.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/integration/test_evaluation_data_proposal.yaml` around lines 95 - 104, The test includes an unnecessary expected_response alongside the turn-level judge metric custom:proposal_evaluation_correctness (defined in turn_metrics and turn_metrics_metadata) which doesn't use ground truth; remove the expected_response block from the test or, if you want to keep it for human-readable documentation, add a short inline comment next to expected_response stating it is documentation-only and not used by the custom:proposal_evaluation_correctness metric so readers aren’t misled.tests/integration/test_proposal_evaluation.py (1)
204-236: ⚡ Quick winTest asserts less than its docstring claims.
The docstring states this verifies that
custom:proposal_evaluation_correctnessruns against the response and the pipeline completes, but the body only checks thatturn.responseis populated — identical totest_full_lifecycle's response check. Nothing confirms the judge metric actually produced a result.Since live LLM scores are nondeterministic, asserting a specific score is fragile, but you can confirm the metric path executed by checking the emitted results (e.g., the JSON output written to
tmp_path / "eval_output") contain acustom:proposal_evaluation_correctnessentry for the turn. This makes the test meaningfully distinct from the lifecycle test.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/integration/test_proposal_evaluation.py` around lines 204 - 236, The test_judge_evaluation currently only asserts that ProposalDriver populated turn.response; update it to also verify that the judge metric ran by reading the evaluation output written to the configured storage (the FileBackend output_dir set to tmp_path / "eval_output") after calling evaluate(system_config, eval_data) and assert that a result entry for custom:proposal_evaluation_correctness exists and is associated with the evaluated turn; locate this logic near the test_judge_evaluation function and use the same identifiers (system_config, evaluate, eval_data, tmp_path, and the turn from eval_data[0].turns[0]) to load the JSON results and assert the presence of the custom:proposal_evaluation_correctness metric for that turn.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/lightspeed_evaluation/pipeline/evaluation/cli.py`:
- Around line 61-75: The KubeCLI.run method can raise subprocess.TimeoutExpired
which escapes callers like KubeCLI.get_resource and ProposalAmender.amend;
modify KubeCLI.run to catch subprocess.TimeoutExpired and normalize it by
returning a failing subprocess.CompletedProcess (non-zero returncode, empty
stdout, stderr describing the timeout and including the timeout value/command)
so callers always receive a CompletedProcess rather than an exception; update
any references in get_resource/ProposalAmender.amend to rely on CompletedProcess
return semantics (or alternatively, raise the project-specific EvaluationError
consistently if your codebase prefers exceptions).
In `@src/lightspeed_evaluation/pipeline/evaluation/proposal_amender.py`:
- Around line 36-39: The try/except in ProposalAmender that calls self._do_amend
currently only catches KeyError/TypeError/ValueError and therefore misses
subprocess errors from self._cli.get_resource (via KubeCLI.run); update the
except clause in ProposalAmender.execute (the block wrapping self._do_amend) to
also catch subprocess.SubprocessError and subprocess.TimeoutExpired (or broaden
to Exception if preferred), or alternatively normalize CLI exceptions inside
KubeCLI.run/_cli.get_resource so they raise a common custom exception that
ProposalAmender can catch; reference _do_amend, ProposalAmender,
_cli.get_resource, KubeCLI.run and ProposalDriver.execute_turn when making the
change.
- Line 80: Remove the stray stdout dump by replacing the print call that outputs
turn_data.response with structured logging: call logger.debug(...) (using the
module logger or create one via logging.getLogger(__name__) if absent) so the
Markdown summary is logged at debug level instead of printed; update the
location where print(turn_data.response) appears in proposal_amender.py (the
code handling turn_data response/amend flow) to use logger.debug and ensure
imports/logger declaration are present.
---
Nitpick comments:
In `@tests/integration/test_evaluation_data_proposal.yaml`:
- Around line 95-104: The test includes an unnecessary expected_response
alongside the turn-level judge metric custom:proposal_evaluation_correctness
(defined in turn_metrics and turn_metrics_metadata) which doesn't use ground
truth; remove the expected_response block from the test or, if you want to keep
it for human-readable documentation, add a short inline comment next to
expected_response stating it is documentation-only and not used by the
custom:proposal_evaluation_correctness metric so readers aren’t misled.
In `@tests/integration/test_proposal_evaluation.py`:
- Around line 204-236: The test_judge_evaluation currently only asserts that
ProposalDriver populated turn.response; update it to also verify that the judge
metric ran by reading the evaluation output written to the configured storage
(the FileBackend output_dir set to tmp_path / "eval_output") after calling
evaluate(system_config, eval_data) and assert that a result entry for
custom:proposal_evaluation_correctness exists and is associated with the
evaluated turn; locate this logic near the test_judge_evaluation function and
use the same identifiers (system_config, evaluate, eval_data, tmp_path, and the
turn from eval_data[0].turns[0]) to load the JSON results and assert the
presence of the custom:proposal_evaluation_correctness metric for that turn.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: e2a40ded-3b7b-4b8b-9ed7-9c9473a5bf72
📒 Files selected for processing (20)
README.mdconfig/system.yamldocs/EVALUATION_GUIDE.mdsrc/lightspeed_evaluation/core/metrics/custom/custom.pysrc/lightspeed_evaluation/core/metrics/custom/prompts.pysrc/lightspeed_evaluation/core/metrics/custom/proposal_eval.pysrc/lightspeed_evaluation/core/models/data.pysrc/lightspeed_evaluation/core/proposal/__init__.pysrc/lightspeed_evaluation/core/proposal/phase.pysrc/lightspeed_evaluation/core/system/validator.pysrc/lightspeed_evaluation/pipeline/evaluation/cli.pysrc/lightspeed_evaluation/pipeline/evaluation/driver.pysrc/lightspeed_evaluation/pipeline/evaluation/proposal_amender.pytests/integration/system-config-agents-proposal.yamltests/integration/test_evaluation_data_proposal.yamltests/integration/test_proposal_evaluation.pytests/unit/core/metrics/custom/test_custom.pytests/unit/core/metrics/custom/test_proposal_eval.pytests/unit/pipeline/evaluation/test_proposal_amender.pytests/unit/pipeline/evaluation/test_proposal_driver.py
rioloc
force-pushed
the
feat/judge-evaluation
branch
2 times, most recently
from
37b4196 to
4a5dc4c
Compare
Extract CLI operations (run, get_resource, apply, delete) into an injectable CLIClient interface with KubeCLI implementation backed by oc/kubectl. ProposalDriver now delegates to KubeCLI instead of internal subprocess calls, enabling dependency injection for the upcoming ProposalAmender. ProposalAmender fetches AnalysisResult, ExecutionResult, VerificationResult, and EscalationResult CRs via CLIClient and populates turn_data.proposal_results with structured status data. It also builds a Markdown workflow summary into turn_data.response. - Add proposal_results field to TurnData model - Create ProposalAmender with CLIClient dependency injection - Integrate ProposalAmender into ProposalDriver (always enabled) - Fallback to _extract_summary if amender fails add custom:proposal_evaluation_correctness LLM-as-judge metric New metric that evaluates agentic remediation workflow quality using an LLM judge. Scores 0.0-1.0 based on diagnosis quality, action appropriateness, risk management, and verification thoroughness. - Add PROPOSAL_EVALUATION_CORRECTNESS_PROMPT template - Register metric in CustomMetrics.supported_metrics - Add METRIC_REQUIREMENTS entry (requires response field) - Add metrics_metadata threshold (0.75) in system.yaml Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
rioloc
force-pushed
the
feat/judge-evaluation
branch
from
4a5dc4c to
20926e7
Compare
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 LLM-as-judge evaluation and enriched data capture for the ProposalDriver evaluation pipeline, along with per-scenario test infrastructure and a new CrashLoopBackOff test fixture.
Core changes
ProposalAmender — after a Proposal CR reaches terminal state, fetches child Result CRs (AnalysisResult, ExecutionResult, VerificationResult, EscalationResult) from the cluster and enriches
TurnDatawith:proposal_results: structured dict with the complete.statusof each child CRresponse: a Markdown workflow summary suitable for both human review and LLM-as-judge evaluationcustom:proposal_evaluation_correctness— a new LLM-as-judge metric (score 0–1) with a multi-dimensional evaluation prompt:_parse_proposal_eval_response): extracts sub-scores and average from the multi-dimensional output formatPer-scenario test infrastructure — setup/cleanup scripts refactored from monolithic per-provider scripts to:
_setup_infra-openai.sh,_setup_infra-claude-vertex.sh) sourced by scenario scriptssetup_oomkill-openai.sh,setup_crashloop_probe-openai.sh, etc.)crashloop-probe-demofixture (nginx with misconfigured liveness probe at/nonexistent-health)Shellcheck compliance —
exportfor variables consumed by sourced scripts (SC2034), exclude SC1091 for dynamicsourcepaths in MakefileWhy
The existing
ProposalDriveronly extracts conditionmessagefields via_extract_summary, losing the rich structured data from child Result CRs. In particular, the Diagnosis from AnalysisResult (root cause analysis, confidence level, detailed summary) is never captured, making it impossible to evaluate whether the agentic workflow diagnosed and remediated the issue correctly.custom:proposal_statusprovides deterministic pass/fail on workflow phase, but cannot assess the quality of diagnosis, actions, or verification. The newproposal_evaluation_correctnessmetric fills this gap using an LLM judge with a structured, multi-dimensional prompt.Design choices
CLIClient abstraction
CLI operations (
run,get_resource,apply,delete) are extracted into aCLIClientABC with aKubeCLIimplementation. BothProposalDriverandProposalAmenderuse the same interface; tests inject a mockCLIClientwithout patching subprocess internals.ProposalAmender as a separate class
Follows the
APIDataAmenderpattern — a dedicated class composed into the driver, responsible for enrichingTurnDatain-place. Navigatesproposal_status.steps.<step>.results[]to readStepResultRefentries, then fetches each child CR viaCLIClient.get_resource().Multi-dimensional judge prompt
The prompt produces per-dimension scores instead of a single holistic score:
Per-scenario scripts
Each scenario (oomkill, crashloop-probe) × provider (openai, claude-vertex) has its own setup/cleanup script that sources a shared
_setup_infra-{provider}.sh/_cleanup_infra-{provider}.sh. This avoids deploying all fixtures for every conversation and makes adding new scenarios mechanical.Relationship between the two proposal metrics
custom:proposal_statusexpected_proposal_statuscustom:proposal_evaluation_correctnessresponse(from amender) +expected_responseThey are complementary:
proposal_statuschecks what happened,proposal_evaluation_correctnesschecks how well it was done.Test plan
test_proposal_driver.pytests pass unchanged (regression)ProposalAmenderunit tests: analysis-only, analysis+execution, full pipeline, failed step, empty results, Markdown summary formattingproposal_evaluation_correctnessmetric unit tests: mock LLM, multi-dimensional score parsing, missing response handling, conversation-level skip, SRE persona verification_parse_proposal_eval_responseparser unit tests: all dimensions, N/A dimensions, fallback average computation, unparseable inputtest_oomkill_full_lifecycle,test_analysis_only,test_oomkill_claude_vertexcrashloop-probe-demofixture and integration eval data for both providersmake pre-commit && make testgreen🤖 Generated with Claude Code