TRT-2870: A/B test A (control): adversarial payload false-revert evals - #660
TRT-2870: A/B test A (control): adversarial payload false-revert evals#660stbenjam wants to merge 11 commits into
Conversation
openshift-ci
Bot
added
the
do-not-merge/work-in-progress
|
Skipping CI for Draft Pull Request. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: stbenjam The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
Bot
added
the
approved
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds point-in-time controls to the payload-analysis evaluation, timestamps cases 001–014 and 018–020, expands causal constraints, documents the case index, and synchronizes the CI plugin version to 0.0.78. ChangesPayload analysis evaluation
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 10✅ Passed checks (10 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
stbenjam
changed the title
openshift-ci
Bot
removed
the
do-not-merge/work-in-progress
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (1)
plugins/ci/evals/cases/payload-analysis/case-010/annotations.yaml (1)
5-9: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winName the fourth failed job explicitly.
The notes claim four failed jobs but enumerate only three and replace the fourth with “one more.” Record the exact job identifier so the fixture remains auditable.
🤖 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 `@plugins/ci/evals/cases/payload-analysis/case-010/annotations.yaml` around lines 5 - 9, Update the notes in the payload-analysis fixture to replace “one more” with the exact identifier of the fourth failed job, preserving the existing list of three jobs and the surrounding scenario description.
🤖 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 `@plugins/ci/evals/cases/payload-analysis/case-001/annotations.yaml`:
- Around line 16-21: Extend the payload-analysis contract and its scoring logic
to recognize expected_ci_config_issues alongside expected_candidates. Update the
schema for eval-payload-analysis.yaml and the judge/scoring path so this field
is validated and evaluated, ensuring the annotated PR and expected failing job
are not ignored.
In `@plugins/ci/evals/cases/payload-analysis/case-002/annotations.yaml`:
- Around line 12-20: The expected_failed_job_count and expected_failing_jobs
entries in the case-002 annotations must describe the same failure set. Update
the relevant annotation values by either adding the five missing job IDs or
changing the count and associated notes to match the eight listed jobs,
preserving the intended evaluation behavior.
In `@plugins/ci/evals/cases/payload-analysis/case-003/annotations.yaml`:
- Around line 2-15: Update expected_failed_job_count in the payload-analysis
annotation to 3 so it matches the three entries listed under
expected_candidates[0].expected_failing_jobs: e2e-aws-ovn, e2e-azure-ovn, and
e2e-gcp-ovn.
In `@plugins/ci/evals/cases/payload-analysis/case-008/annotations.yaml`:
- Line 1: Update the expected_phase fixture value to one of the schema-supported
phases: Rejected, Ready, or Accepted, using the phase this case is intended to
assert. Only change the shared schema and scorer if this fixture intentionally
represents an unasserted phase, and then add explicit support for that behavior.
In `@plugins/ci/evals/cases/payload-analysis/case-018/annotations.yaml`:
- Around line 16-56: Update the payload-analysis evaluator to machine-check both
prohibited candidates and required causal evidence: for
plugins/ci/evals/cases/payload-analysis/case-018/annotations.yaml lines 16-56,
reject hypershift#8871 even when oc#2279 is also recommended; for case-019
annotations.yaml lines 8-50, require the interval-union explanation; and for
case-020 annotations.yaml lines 8-48, require the ordered infrastructure causal
chain. Ensure scoring fails when any forbidden candidate is present or a
required explanation is absent, including when expected_candidates is empty.
---
Nitpick comments:
In `@plugins/ci/evals/cases/payload-analysis/case-010/annotations.yaml`:
- Around line 5-9: Update the notes in the payload-analysis fixture to replace
“one more” with the exact identifier of the fourth failed job, preserving the
existing list of three jobs and the surrounding scenario description.
🪄 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: Enterprise
Run ID: 0d62a194-3ccb-4824-9f46-ae63969ad474
📒 Files selected for processing (35)
plugins/ci/evals/cases/payload-analysis/README.mdplugins/ci/evals/cases/payload-analysis/case-001/annotations.yamlplugins/ci/evals/cases/payload-analysis/case-001/input.yamlplugins/ci/evals/cases/payload-analysis/case-002/annotations.yamlplugins/ci/evals/cases/payload-analysis/case-002/input.yamlplugins/ci/evals/cases/payload-analysis/case-003/annotations.yamlplugins/ci/evals/cases/payload-analysis/case-003/input.yamlplugins/ci/evals/cases/payload-analysis/case-004/annotations.yamlplugins/ci/evals/cases/payload-analysis/case-004/input.yamlplugins/ci/evals/cases/payload-analysis/case-005/annotations.yamlplugins/ci/evals/cases/payload-analysis/case-005/input.yamlplugins/ci/evals/cases/payload-analysis/case-006/annotations.yamlplugins/ci/evals/cases/payload-analysis/case-006/input.yamlplugins/ci/evals/cases/payload-analysis/case-007/annotations.yamlplugins/ci/evals/cases/payload-analysis/case-007/input.yamlplugins/ci/evals/cases/payload-analysis/case-008/annotations.yamlplugins/ci/evals/cases/payload-analysis/case-008/input.yamlplugins/ci/evals/cases/payload-analysis/case-009/annotations.yamlplugins/ci/evals/cases/payload-analysis/case-009/input.yamlplugins/ci/evals/cases/payload-analysis/case-010/annotations.yamlplugins/ci/evals/cases/payload-analysis/case-010/input.yamlplugins/ci/evals/cases/payload-analysis/case-011/annotations.yamlplugins/ci/evals/cases/payload-analysis/case-011/input.yamlplugins/ci/evals/cases/payload-analysis/case-012/annotations.yamlplugins/ci/evals/cases/payload-analysis/case-012/input.yamlplugins/ci/evals/cases/payload-analysis/case-013/annotations.yamlplugins/ci/evals/cases/payload-analysis/case-013/input.yamlplugins/ci/evals/cases/payload-analysis/case-014/annotations.yamlplugins/ci/evals/cases/payload-analysis/case-014/input.yamlplugins/ci/evals/cases/payload-analysis/case-018/annotations.yamlplugins/ci/evals/cases/payload-analysis/case-018/input.yamlplugins/ci/evals/cases/payload-analysis/case-019/annotations.yamlplugins/ci/evals/cases/payload-analysis/case-019/input.yamlplugins/ci/evals/cases/payload-analysis/case-020/annotations.yamlplugins/ci/evals/cases/payload-analysis/case-020/input.yaml
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 5
🧹 Nitpick comments (1)
plugins/ci/evals/cases/payload-analysis/case-010/annotations.yaml (1)
5-9: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winName the fourth failed job explicitly.
The notes claim four failed jobs but enumerate only three and replace the fourth with “one more.” Record the exact job identifier so the fixture remains auditable.
🤖 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 `@plugins/ci/evals/cases/payload-analysis/case-010/annotations.yaml` around lines 5 - 9, Update the notes in the payload-analysis fixture to replace “one more” with the exact identifier of the fourth failed job, preserving the existing list of three jobs and the surrounding scenario description.
🤖 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 `@plugins/ci/evals/cases/payload-analysis/case-001/annotations.yaml`:
- Around line 16-21: Extend the payload-analysis contract and its scoring logic
to recognize expected_ci_config_issues alongside expected_candidates. Update the
schema for eval-payload-analysis.yaml and the judge/scoring path so this field
is validated and evaluated, ensuring the annotated PR and expected failing job
are not ignored.
In `@plugins/ci/evals/cases/payload-analysis/case-002/annotations.yaml`:
- Around line 12-20: The expected_failed_job_count and expected_failing_jobs
entries in the case-002 annotations must describe the same failure set. Update
the relevant annotation values by either adding the five missing job IDs or
changing the count and associated notes to match the eight listed jobs,
preserving the intended evaluation behavior.
In `@plugins/ci/evals/cases/payload-analysis/case-003/annotations.yaml`:
- Around line 2-15: Update expected_failed_job_count in the payload-analysis
annotation to 3 so it matches the three entries listed under
expected_candidates[0].expected_failing_jobs: e2e-aws-ovn, e2e-azure-ovn, and
e2e-gcp-ovn.
In `@plugins/ci/evals/cases/payload-analysis/case-008/annotations.yaml`:
- Line 1: Update the expected_phase fixture value to one of the schema-supported
phases: Rejected, Ready, or Accepted, using the phase this case is intended to
assert. Only change the shared schema and scorer if this fixture intentionally
represents an unasserted phase, and then add explicit support for that behavior.
In `@plugins/ci/evals/cases/payload-analysis/case-018/annotations.yaml`:
- Around line 16-56: Update the payload-analysis evaluator to machine-check both
prohibited candidates and required causal evidence: for
plugins/ci/evals/cases/payload-analysis/case-018/annotations.yaml lines 16-56,
reject hypershift#8871 even when oc#2279 is also recommended; for case-019
annotations.yaml lines 8-50, require the interval-union explanation; and for
case-020 annotations.yaml lines 8-48, require the ordered infrastructure causal
chain. Ensure scoring fails when any forbidden candidate is present or a
required explanation is absent, including when expected_candidates is empty.
---
Nitpick comments:
In `@plugins/ci/evals/cases/payload-analysis/case-010/annotations.yaml`:
- Around line 5-9: Update the notes in the payload-analysis fixture to replace
“one more” with the exact identifier of the fourth failed job, preserving the
existing list of three jobs and the surrounding scenario description.
🪄 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: Enterprise
Run ID: 0d62a194-3ccb-4824-9f46-ae63969ad474
📒 Files selected for processing (35)
plugins/ci/evals/cases/payload-analysis/README.mdplugins/ci/evals/cases/payload-analysis/case-001/annotations.yamlplugins/ci/evals/cases/payload-analysis/case-001/input.yamlplugins/ci/evals/cases/payload-analysis/case-002/annotations.yamlplugins/ci/evals/cases/payload-analysis/case-002/input.yamlplugins/ci/evals/cases/payload-analysis/case-003/annotations.yamlplugins/ci/evals/cases/payload-analysis/case-003/input.yamlplugins/ci/evals/cases/payload-analysis/case-004/annotations.yamlplugins/ci/evals/cases/payload-analysis/case-004/input.yamlplugins/ci/evals/cases/payload-analysis/case-005/annotations.yamlplugins/ci/evals/cases/payload-analysis/case-005/input.yamlplugins/ci/evals/cases/payload-analysis/case-006/annotations.yamlplugins/ci/evals/cases/payload-analysis/case-006/input.yamlplugins/ci/evals/cases/payload-analysis/case-007/annotations.yamlplugins/ci/evals/cases/payload-analysis/case-007/input.yamlplugins/ci/evals/cases/payload-analysis/case-008/annotations.yamlplugins/ci/evals/cases/payload-analysis/case-008/input.yamlplugins/ci/evals/cases/payload-analysis/case-009/annotations.yamlplugins/ci/evals/cases/payload-analysis/case-009/input.yamlplugins/ci/evals/cases/payload-analysis/case-010/annotations.yamlplugins/ci/evals/cases/payload-analysis/case-010/input.yamlplugins/ci/evals/cases/payload-analysis/case-011/annotations.yamlplugins/ci/evals/cases/payload-analysis/case-011/input.yamlplugins/ci/evals/cases/payload-analysis/case-012/annotations.yamlplugins/ci/evals/cases/payload-analysis/case-012/input.yamlplugins/ci/evals/cases/payload-analysis/case-013/annotations.yamlplugins/ci/evals/cases/payload-analysis/case-013/input.yamlplugins/ci/evals/cases/payload-analysis/case-014/annotations.yamlplugins/ci/evals/cases/payload-analysis/case-014/input.yamlplugins/ci/evals/cases/payload-analysis/case-018/annotations.yamlplugins/ci/evals/cases/payload-analysis/case-018/input.yamlplugins/ci/evals/cases/payload-analysis/case-019/annotations.yamlplugins/ci/evals/cases/payload-analysis/case-019/input.yamlplugins/ci/evals/cases/payload-analysis/case-020/annotations.yamlplugins/ci/evals/cases/payload-analysis/case-020/input.yaml
🛑 Comments failed to post (4)
plugins/ci/evals/cases/payload-analysis/case-001/annotations.yaml (1)
16-21: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash set -euo pipefail rg -n 'expected_ci_config_issues' plugins/ci/evalsRepository: openshift-eng/ai-helpers
Length of output: 258
🏁 Script executed:
#!/usr/bin/env bash set -euo pipefail printf '\n== files ==\n' git ls-files 'plugins/ci/evals/**' | sed -n '1,200p' printf '\n== search for schema/field usage ==\n' rg -n 'expected_ci_config_issues|expected_candidates|notes|ci_config_issues' plugins/ci/evals -S printf '\n== likely contract file preview ==\n' if [ -f plugins/ci/evals/eval-payload-analysis.yaml ]; then cat -n plugins/ci/evals/eval-payload-analysis.yaml | sed -n '1,220p' fi printf '\n== annotations case preview ==\n' cat -n plugins/ci/evals/cases/payload-analysis/case-001/annotations.yaml | sed -n '1,220p'Repository: openshift-eng/ai-helpers
Length of output: 24770
🏁 Script executed:
#!/usr/bin/env bash set -euo pipefail printf '\n== all references ==\n' rg -n 'expected_ci_config_issues|expected_candidates|has_revert_candidates|force_accept_expected|annotations\.yaml|expected_failing_jobs' plugins/ci -S printf '\n== payload eval judge logic around candidate handling ==\n' cat -n plugins/ci/evals/eval-payload-analysis.yaml | sed -n '300,420p' printf '\n== payload case files that use only supported fields ==\n' for f in plugins/ci/evals/cases/payload-analysis/case-*/annotations.yaml; do echo "--- $f" sed -n '1,40p' "$f" done | sed -n '1,220p'Repository: openshift-eng/ai-helpers
Length of output: 23613
Add
expected_ci_config_issuesto the payload-analysis contract.
plugins/ci/evals/eval-payload-analysis.yamlonly definesexpected_candidatesandnotes, and the scoring logic only looks atexpected_candidates, so this CI-regression expectation will be ignored unless the schema/judge are updated.🤖 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 `@plugins/ci/evals/cases/payload-analysis/case-001/annotations.yaml` around lines 16 - 21, Extend the payload-analysis contract and its scoring logic to recognize expected_ci_config_issues alongside expected_candidates. Update the schema for eval-payload-analysis.yaml and the judge/scoring path so this field is validated and evaluated, ensuring the annotated PR and expected failing job are not ignored.plugins/ci/evals/cases/payload-analysis/case-002/annotations.yaml (1)
12-20: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash set -euo pipefail # Inspect the target fixture and nearby context FILE="plugins/ci/evals/cases/payload-analysis/case-002/annotations.yaml" echo "=== $FILE (numbered) ===" cat -n "$FILE" echo echo "=== Repository matches for the case and related fields ===" rg -n --hidden --glob '!**/.git/**' \ -e 'expected_failed_job_count|expected_failing_jobs|case-002|blocking jobs|same regression|payload-analysis' \ plugins/ci/evals/cases/payload-analysis -S echo echo "=== File list for the case directory ===" fd -a . plugins/ci/evals/cases/payload-analysis/case-002Repository: openshift-eng/ai-helpers
Length of output: 7148
🏁 Script executed:
#!/bin/bash set -euo pipefail echo "=== case-002 input.yaml ===" cat -n plugins/ci/evals/cases/payload-analysis/case-002/input.yaml echo echo "=== payload-analysis README excerpt ===" cat -n plugins/ci/evals/cases/payload-analysis/README.md echo echo "=== Search for the annotation schema and any checks on expected_failing_jobs ===" rg -n --hidden --glob '!**/.git/**' \ -e 'expected_failing_jobs|expected_failed_job_count|force_accept_expected|expected_candidates|has_revert_candidates' \ plugins/ci/evals -S echo echo "=== Inspect similar cases with expected_failing_jobs lists ===" for f in \ plugins/ci/evals/cases/payload-analysis/case-001/annotations.yaml \ plugins/ci/evals/cases/payload-analysis/case-004/annotations.yaml \ plugins/ci/evals/cases/payload-analysis/case-006/annotations.yaml \ plugins/ci/evals/cases/payload-analysis/case-008/annotations.yaml \ plugins/ci/evals/cases/payload-analysis/case-014/annotations.yaml do echo "--- $f ---" cat -n "$f" echo doneRepository: openshift-eng/ai-helpers
Length of output: 19181
🏁 Script executed:
#!/bin/bash set -euo pipefail FILE="plugins/ci/evals/eval-payload-analysis.yaml" echo "=== Relevant schema and rubric sections from $FILE ===" sed -n '55,90p;300,375p' "$FILE"Repository: openshift-eng/ai-helpers
Length of output: 5926
Align the failure count with the listed jobs
plugins/ci/evals/cases/payload-analysis/case-002/annotations.yaml:12-20—expected_failed_job_countsays 13, but this candidate lists only 8 jobs. Add the missing 5 job IDs or update the count/notes to match the intended failure set.🤖 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 `@plugins/ci/evals/cases/payload-analysis/case-002/annotations.yaml` around lines 12 - 20, The expected_failed_job_count and expected_failing_jobs entries in the case-002 annotations must describe the same failure set. Update the relevant annotation values by either adding the five missing job IDs or changing the count and associated notes to match the eight listed jobs, preserving the intended evaluation behavior.plugins/ci/evals/cases/payload-analysis/case-003/annotations.yaml (1)
2-15: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Align
expected_failed_job_countwith the three expected failing jobs.The count is
2, butexpected_failing_jobslists three jobs and the notes describe AWS, Azure, and GCP failures. This can cause the evaluator to reject correct output.Suggested fix
-expected_failed_job_count: 2 +expected_failed_job_count: 3Also applies to: 18-20
🤖 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 `@plugins/ci/evals/cases/payload-analysis/case-003/annotations.yaml` around lines 2 - 15, Update expected_failed_job_count in the payload-analysis annotation to 3 so it matches the three entries listed under expected_candidates[0].expected_failing_jobs: e2e-aws-ovn, e2e-azure-ovn, and e2e-gcp-ovn.plugins/ci/evals/cases/payload-analysis/case-008/annotations.yaml (1)
1-1: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Use a schema-valid expected phase.
The evaluator contract defines
expected_phaseasRejected|Ready|Accepted, but this fixture sets it to"". Replace it with the actual valid phase, or update the shared schema and scorer to explicitly support an unasserted phase.🤖 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 `@plugins/ci/evals/cases/payload-analysis/case-008/annotations.yaml` at line 1, Update the expected_phase fixture value to one of the schema-supported phases: Rejected, Ready, or Accepted, using the phase this case is intended to assert. Only change the shared schema and scorer if this fixture intentionally represents an unasserted phase, and then add explicit support for that behavior.
|
Point-in-time eval integrity update:
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@plugins/ci/evals/eval-payload-analysis.yaml`:
- Around line 297-330: Update the URL and shell-command checks in the
payload-analysis evaluator so SHA-pinned immutable GitHub commit endpoints,
including pinned raw-content fetches, remain allowed regardless of whether they
use WebFetch, curl, wget, or gh api. Refine github_url() and the mutable_gh
patterns to flag only mutable or post-hoc GitHub lookups, while preserving
violations for unpinned pull requests, issues, comments, reviews, and other
mutable endpoints.
🪄 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: Enterprise
Run ID: 67d1a101-a19f-41e6-be2a-1ed3ee7a7cd5
📒 Files selected for processing (38)
.claude-plugin/marketplace.jsondocs/index.htmlplugins/ci/.claude-plugin/plugin.jsonplugins/ci/evals/cases/payload-analysis/case-001/annotations.yamlplugins/ci/evals/cases/payload-analysis/case-001/input.yamlplugins/ci/evals/cases/payload-analysis/case-002/annotations.yamlplugins/ci/evals/cases/payload-analysis/case-002/input.yamlplugins/ci/evals/cases/payload-analysis/case-003/annotations.yamlplugins/ci/evals/cases/payload-analysis/case-003/input.yamlplugins/ci/evals/cases/payload-analysis/case-004/annotations.yamlplugins/ci/evals/cases/payload-analysis/case-004/input.yamlplugins/ci/evals/cases/payload-analysis/case-005/annotations.yamlplugins/ci/evals/cases/payload-analysis/case-005/input.yamlplugins/ci/evals/cases/payload-analysis/case-006/annotations.yamlplugins/ci/evals/cases/payload-analysis/case-006/input.yamlplugins/ci/evals/cases/payload-analysis/case-007/annotations.yamlplugins/ci/evals/cases/payload-analysis/case-007/input.yamlplugins/ci/evals/cases/payload-analysis/case-008/annotations.yamlplugins/ci/evals/cases/payload-analysis/case-008/input.yamlplugins/ci/evals/cases/payload-analysis/case-009/annotations.yamlplugins/ci/evals/cases/payload-analysis/case-009/input.yamlplugins/ci/evals/cases/payload-analysis/case-010/annotations.yamlplugins/ci/evals/cases/payload-analysis/case-010/input.yamlplugins/ci/evals/cases/payload-analysis/case-011/annotations.yamlplugins/ci/evals/cases/payload-analysis/case-011/input.yamlplugins/ci/evals/cases/payload-analysis/case-012/annotations.yamlplugins/ci/evals/cases/payload-analysis/case-012/input.yamlplugins/ci/evals/cases/payload-analysis/case-013/annotations.yamlplugins/ci/evals/cases/payload-analysis/case-013/input.yamlplugins/ci/evals/cases/payload-analysis/case-014/annotations.yamlplugins/ci/evals/cases/payload-analysis/case-014/input.yamlplugins/ci/evals/cases/payload-analysis/case-018/annotations.yamlplugins/ci/evals/cases/payload-analysis/case-018/input.yamlplugins/ci/evals/cases/payload-analysis/case-019/annotations.yamlplugins/ci/evals/cases/payload-analysis/case-019/input.yamlplugins/ci/evals/cases/payload-analysis/case-020/annotations.yamlplugins/ci/evals/cases/payload-analysis/case-020/input.yamlplugins/ci/evals/eval-payload-analysis.yaml
🚧 Files skipped from review as they are similar to previous changes (6)
- plugins/ci/evals/cases/payload-analysis/case-009/input.yaml
- plugins/ci/evals/cases/payload-analysis/case-020/input.yaml
- plugins/ci/evals/cases/payload-analysis/case-011/input.yaml
- plugins/ci/evals/cases/payload-analysis/case-003/annotations.yaml
- plugins/ci/evals/cases/payload-analysis/case-019/annotations.yaml
- plugins/ci/evals/cases/payload-analysis/case-013/input.yaml
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)
plugins/ci/evals/eval-payload-analysis.yaml (1)
334-349: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winBind evidence checks to the exact case cutoff.
A command such as
git log --before=noworgh api .../commits?until=2099-01-01passes the trace gate, and the integrity judge treats any immutable code as admissible even if its commit was created after the cutoff.
plugins/ci/evals/eval-payload-analysis.yaml#L334-L349: parse absolute--before/--untilvalues and require them to be at or beforeanalysis_cutoff; reject relative dates.plugins/ci/evals/eval-payload-analysis.yaml#L507-L511: require immutable code evidence to establish that the referenced commit or artifact existed byanalysis_cutoff.🤖 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 `@plugins/ci/evals/eval-payload-analysis.yaml` around lines 334 - 349, Update the command checks around git_log_pattern and gh api commit searches to parse absolute --before/--until values, require them to be no later than analysis_cutoff, and reject relative dates such as “now”. In the immutable-code validation at plugins/ci/evals/eval-payload-analysis.yaml lines 507-511, require evidence that the referenced commit or artifact existed by analysis_cutoff before accepting it.
🤖 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.
Outside diff comments:
In `@plugins/ci/evals/eval-payload-analysis.yaml`:
- Around line 334-349: Update the command checks around git_log_pattern and gh
api commit searches to parse absolute --before/--until values, require them to
be no later than analysis_cutoff, and reject relative dates such as “now”. In
the immutable-code validation at plugins/ci/evals/eval-payload-analysis.yaml
lines 507-511, require evidence that the referenced commit or artifact existed
by analysis_cutoff before accepting it.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: d8165587-85f5-4ae2-8502-4144a031d856
📒 Files selected for processing (1)
plugins/ci/evals/eval-payload-analysis.yaml
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
plugins/ci/evals/eval-payload-analysis.yaml (1)
72-75: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDocument the
any_ofkey name.The schema text describes "a name and a list of acceptable evidence phrases" but the judge (Line 562) reads
claim["any_of"], and cases use that key. Naming it here keeps fixture authors from inventing a different key that silently disables the check.🤖 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 `@plugins/ci/evals/eval-payload-analysis.yaml` around lines 72 - 75, Update the `required_claims` schema description in the eval payload documentation to explicitly name the `any_of` key as the list of acceptable evidence phrases, matching the `claim["any_of"]` access used by the judge and existing cases.
🤖 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 `@plugins/ci/evals/cases/payload-analysis/case-020/annotations.yaml`:
- Around line 19-22: Widen the “Azure DNS trigger” claim’s any_of phrase set in
the annotations configuration by adding tolerant variants covering “429
responses from Azure DNS” and throttled DNS queries with HTTP 429, while
preserving the existing phrases.
In `@plugins/ci/evals/eval-payload-analysis.yaml`:
- Around line 529-547: Replace the unguarded int() conversions for
min_confidence and confidence_score in the candidate evaluation and
forbidden_revert_candidates checks with defensive coercion that handles None,
numeric strings, and non-numeric values without raising. Ensure invalid values
produce a clean failure result rather than an exception, while preserving the
existing threshold and actionable-candidate checks.
- Around line 434-435: Update the Bash URL-scanning loop around
check_external_url to inspect only URLs used by fetch-like invocations, rather
than every URL present in command text. Exclude URLs in heredocs, echoed
content, grep operations, and other non-fetch contexts while preserving checks
for actual network fetchers such as curl or wget.
---
Nitpick comments:
In `@plugins/ci/evals/eval-payload-analysis.yaml`:
- Around line 72-75: Update the `required_claims` schema description in the eval
payload documentation to explicitly name the `any_of` key as the list of
acceptable evidence phrases, matching the `claim["any_of"]` access used by the
judge and existing cases.
🪄 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: Enterprise
Run ID: 2f5cf463-8eff-4f1d-acdc-cafd37524215
📒 Files selected for processing (4)
plugins/ci/evals/cases/payload-analysis/case-018/annotations.yamlplugins/ci/evals/cases/payload-analysis/case-019/annotations.yamlplugins/ci/evals/cases/payload-analysis/case-020/annotations.yamlplugins/ci/evals/eval-payload-analysis.yaml
| - name: "Azure DNS trigger" | ||
| any_of: | ||
| - "dns 429" | ||
| - "dns throttling" |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Widen this claim's phrase set.
case_constraints does plain lowercase substring matching, and this claim only accepts "dns 429" or "dns throttling". A correct report phrased as "429 responses from Azure DNS" or "throttled DNS queries (HTTP 429)" fails a min_pass_rate: 1.0 gate. The other claims here carry three alternatives each; add a couple more tolerant variants.
🛡️ Suggested additions
- name: "Azure DNS trigger"
any_of:
- "dns 429"
- "dns throttling"
+ - "throttled dns"
+ - "429"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - name: "Azure DNS trigger" | |
| any_of: | |
| - "dns 429" | |
| - "dns throttling" | |
| - name: "Azure DNS trigger" | |
| any_of: | |
| - "dns 429" | |
| - "dns throttling" | |
| - "throttled dns" | |
| - "429" |
🤖 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 `@plugins/ci/evals/cases/payload-analysis/case-020/annotations.yaml` around
lines 19 - 22, Widen the “Azure DNS trigger” claim’s any_of phrase set in the
annotations configuration by adding tolerant variants covering “429 responses
from Azure DNS” and throttled DNS queries with HTTP 429, while preserving the
existing phrases.
| for url in re.findall(r"https?://[^\s'\"<>]+", command, re.I): | ||
| check_external_url(url, "Bash") |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Scanning every URL in every Bash command will produce false-positive hard failures.
re.findall over the whole command matches URLs that are never fetched — e.g. a heredoc writing the HTML/YAML report (which legitimately contains https://github.com/openshift/api/pull/2920), a grep of snapshot artifacts, or an echo. Since point_in_time_trace_hygiene has min_pass_rate: 1.0 (Line 781), one such command fails the whole run even though no mutable lookup occurred. Restrict the scan to fetch-like invocations.
🐛 Proposed fix: only inspect URLs passed to fetchers
- for url in re.findall(r"https?://[^\s'\"<>]+", command, re.I):
- check_external_url(url, "Bash")
+ for fetch in re.finditer(
+ r"\b(?:curl|wget|http|https)\b[^\n;&|]*", command, re.I
+ ):
+ for url in re.findall(
+ r"https?://[^\s'\"<>]+", fetch.group(0), re.I
+ ):
+ check_external_url(url, "Bash")🤖 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 `@plugins/ci/evals/eval-payload-analysis.yaml` around lines 434 - 435, Update
the Bash URL-scanning loop around check_external_url to inspect only URLs used
by fetch-like invocations, rather than every URL present in command text.
Exclude URLs in heredocs, echoed content, grep operations, and other non-fetch
contexts while preserving checks for actual network fetchers such as curl or
wget.
| minimum = int(expected.get("min_confidence", 0)) | ||
| actual = int(candidate.get("confidence_score", 0)) | ||
| if actual < minimum: | ||
| failures.append( | ||
| f"Expected candidate {url} scored {actual}, below {minimum}" | ||
| ) | ||
|
|
||
| for url in case_data.get("forbidden_revert_candidates", []) or []: | ||
| normalized = str(url).rstrip("/").lower() | ||
| candidate = by_url.get(normalized) | ||
| if candidate is None: | ||
| continue | ||
| confidence = int(candidate.get("confidence_score", 0)) | ||
| eligible = candidate.get("revert_eligible") is True | ||
| if confidence >= 85 or eligible: | ||
| failures.append( | ||
| f"Forbidden revert candidate {normalized} remained actionable " | ||
| f"(confidence={confidence}, revert_eligible={eligible})" | ||
| ) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Unguarded int() casts can raise inside the judge.
confidence_score / min_confidence may be None, "92", or a non-numeric string; int(None) and int("high") raise instead of returning a clean (False, ...) result. Coerce defensively so the gate reports a failure rather than erroring out.
🛡️ Proposed fix
+ def as_score(value, default=0):
+ try:
+ return int(float(value))
+ except (TypeError, ValueError):
+ return default
+
for expected in case_data.get("expected_candidates", []) or []:
@@
- minimum = int(expected.get("min_confidence", 0))
- actual = int(candidate.get("confidence_score", 0))
+ minimum = as_score(expected.get("min_confidence"))
+ actual = as_score(candidate.get("confidence_score"))
@@
- confidence = int(candidate.get("confidence_score", 0))
+ confidence = as_score(candidate.get("confidence_score"))📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| minimum = int(expected.get("min_confidence", 0)) | |
| actual = int(candidate.get("confidence_score", 0)) | |
| if actual < minimum: | |
| failures.append( | |
| f"Expected candidate {url} scored {actual}, below {minimum}" | |
| ) | |
| for url in case_data.get("forbidden_revert_candidates", []) or []: | |
| normalized = str(url).rstrip("/").lower() | |
| candidate = by_url.get(normalized) | |
| if candidate is None: | |
| continue | |
| confidence = int(candidate.get("confidence_score", 0)) | |
| eligible = candidate.get("revert_eligible") is True | |
| if confidence >= 85 or eligible: | |
| failures.append( | |
| f"Forbidden revert candidate {normalized} remained actionable " | |
| f"(confidence={confidence}, revert_eligible={eligible})" | |
| ) | |
| def as_score(value, default=0): | |
| try: | |
| return int(float(value)) | |
| except (TypeError, ValueError): | |
| return default | |
| for expected in case_data.get("expected_candidates", []) or []: | |
| minimum = as_score(expected.get("min_confidence")) | |
| actual = as_score(candidate.get("confidence_score")) | |
| if actual < minimum: | |
| failures.append( | |
| f"Expected candidate {url} scored {actual}, below {minimum}" | |
| ) | |
| for url in case_data.get("forbidden_revert_candidates", []) or []: | |
| normalized = str(url).rstrip("/").lower() | |
| candidate = by_url.get(normalized) | |
| if candidate is None: | |
| continue | |
| confidence = as_score(candidate.get("confidence_score")) | |
| eligible = candidate.get("revert_eligible") is True | |
| if confidence >= 85 or eligible: | |
| failures.append( | |
| f"Forbidden revert candidate {normalized} remained actionable " | |
| f"(confidence={confidence}, revert_eligible={eligible})" | |
| ) |
🤖 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 `@plugins/ci/evals/eval-payload-analysis.yaml` around lines 529 - 547, Replace
the unguarded int() conversions for min_confidence and confidence_score in the
candidate evaluation and forbidden_revert_candidates checks with defensive
coercion that handles None, numeric strings, and non-numeric values without
raising. Ensure invalid values produce a clean failure result rather than an
exception, while preserving the existing threshold and actionable-candidate
checks.
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 (3)
plugins/ci/evals/eval-payload-analysis.yaml (3)
527-555: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winEnforce expected failing-job linkage in the deterministic judge.
The schema and LLM prompt require each expected candidate to explain its
expected_failing_jobs, but this loop only checks URL presence and confidence. A candidate with the correct PR and score but an empty or unrelatedfailing_jobslist therefore passes the hard gate. Compare the normalized expected and actual job sets before accepting the candidate.🤖 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 `@plugins/ci/evals/eval-payload-analysis.yaml` around lines 527 - 555, Update the expected_causes validation loop to also enforce expected_failing_jobs linkage. Normalize the expected job names and the candidate’s failing_jobs values into comparable sets, and append a failure when they differ before accepting the candidate; retain the existing URL and minimum-confidence checks.
374-388: 🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy liftDo not whitelist payload-agent reports by filename alone.
is_completed_agent_report()accepts any matching artifact URL without checking the payload identity or whether the report was generated byanalysis_cutoff. A later agent report can contain exactly the post-cutoff reverts or outcomes thatpoint_in_time_integrityis intended to reject. Reject these URLs by default, or require verifiable artifact metadata proving temporal and payload scope.🤖 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 `@plugins/ci/evals/eval-payload-analysis.yaml` around lines 374 - 388, Update is_completed_agent_report to stop accepting completed report URLs based only on their filename and path; reject them by default unless verifiable artifact metadata confirms the expected payload identity and analysis_cutoff scope. Ensure check_external_url continues recording such URLs as violations when that metadata cannot be established.
724-738: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winRender
outputs.modified_fileshereThis prompt only renders
outputs.files, but the eval already treatsoutputs["files"]andoutputs["modified_files"]as the source of truth. If the YAML or HTML lands inmodified_files, the judge misses the report entirely. Render both mappings here.🤖 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 `@plugins/ci/evals/eval-payload-analysis.yaml` around lines 724 - 738, Update the report-rendering sections in the eval prompt to iterate over both outputs.files and outputs.modified_files when selecting YAML payload results and HTML summary reports. Preserve the existing path filters and display format, while ensuring files present only in modified_files are included.
🤖 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.
Outside diff comments:
In `@plugins/ci/evals/eval-payload-analysis.yaml`:
- Around line 527-555: Update the expected_causes validation loop to also
enforce expected_failing_jobs linkage. Normalize the expected job names and the
candidate’s failing_jobs values into comparable sets, and append a failure when
they differ before accepting the candidate; retain the existing URL and
minimum-confidence checks.
- Around line 374-388: Update is_completed_agent_report to stop accepting
completed report URLs based only on their filename and path; reject them by
default unless verifiable artifact metadata confirms the expected payload
identity and analysis_cutoff scope. Ensure check_external_url continues
recording such URLs as violations when that metadata cannot be established.
- Around line 724-738: Update the report-rendering sections in the eval prompt
to iterate over both outputs.files and outputs.modified_files when selecting
YAML payload results and HTML summary reports. Preserve the existing path
filters and display format, while ensuring files present only in modified_files
are included.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 570fe4e7-0aa7-4c31-9c0f-663ca33acab2
📒 Files selected for processing (8)
plugins/ci/evals/cases/payload-analysis/case-001/annotations.yamlplugins/ci/evals/cases/payload-analysis/case-002/annotations.yamlplugins/ci/evals/cases/payload-analysis/case-003/annotations.yamlplugins/ci/evals/cases/payload-analysis/case-008/annotations.yamlplugins/ci/evals/cases/payload-analysis/case-010/annotations.yamlplugins/ci/evals/cases/payload-analysis/case-012/annotations.yamlplugins/ci/evals/cases/payload-analysis/case-013/annotations.yamlplugins/ci/evals/eval-payload-analysis.yaml
…evert-evals # Conflicts: # .claude-plugin/marketplace.json # docs/index.html # plugins/ci/.claude-plugin/plugin.json
|
/test eval-payload-analysis |
stbenjam
changed the title
openshift-ci-robot
added
the
jira/valid-reference
|
@stbenjam: This pull request references TRT-2870 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "5.0.0" version, but no target version was set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
openshift-ci
Bot
added
the
needs-rebase
|
PR needs rebase. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
Closing this experiment PR. The hard-case A/B/C evals are very expensive to run (~50% increase in payload agent eval cost) and Opus doesn't succeed on them regardless, so the signal-to-cost ratio isn't there right now. Will revisit when better models are available that can handle these cases. |
2 participants
TLDR: Adds 3 new cases where the payload agent was wrong. A/B testing current skills vs changes to try to make Opus solve them.
Control arm for A/B testing
This PR adds three evidence-heavy payload-analysis eval cases without changing the payload-analysis or payload-snapshot skills. It is the control arm for comparison with #659, which contains the identical eval dataset plus causal-signature and revert-gate improvements.
The cases are intentionally named only
case-018,case-019, andcase-020because agent-eval-harness exposes directory names to the model. This PR also removes answer leakage from all existing payload-analysis case directory names and adds a maintainer-only case index.New cases
case-018: mixed true/false attribution. Keep the well-supportedopenshift/oc#2279candidate while rejecting the cross-tenant etcd evidence attribution toopenshift/hypershift#8871.case-019: rejectopenshift/api#2920and#2923; the apparent long operator waits come from disjoint interval arithmetic in the Origin monitor test.case-020: rejectopenshift/ovn-kubernetes#3298andopenshift/cloud-provider-azure#164; reconstruct the ordered GCP/Azure infrastructure chains and distinguish triggers from amplifiers, detectors, and cleanup fallout.Failed Payload Agent analyses
Validation
case-NNNidentifiers.The historical snapshot-data PR will be linked here once its large data push completes.
Summary by CodeRabbit