Add 3CMO false positive eval for payload-analysis - #626
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: theobarberbany 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
WalkthroughThis change adds artifact-grounded payload-analysis cases for CCCMO false positives and missing data, updates judge scoring rules, and records missing JUnit or build-log artifacts as data gaps. ChangesPayload analysis grounding
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related issues
🚥 Pre-merge checks | ✅ 10✅ Passed checks (10 passed)
✨ 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 |
d09c44a to
ad1d5fa
Compare
ad1d5fa to
a479533
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
plugins/ci/skills/payload-snapshot/scripts/payload_snapshot.py (1)
1472-1474: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse
setdefaultfor cleaner dictionary assignment.Both blocks initializing and appending to
data_gapsuse a redundantget()and reassignment pattern. This can be simplified using Python'ssetdefaultmethod, which is more concise and idiomatic.
plugins/ci/skills/payload-snapshot/scripts/payload_snapshot.py#L1472-L1474: Replace withentry.setdefault("data_gaps", []).append("junit")plugins/ci/skills/payload-snapshot/scripts/payload_snapshot.py#L1488-L1490: Replace withentry.setdefault("data_gaps", []).append("build_log")🤖 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/skills/payload-snapshot/scripts/payload_snapshot.py` around lines 1472 - 1474, In the failure-handling blocks of the payload snapshot logic, replace the redundant data_gaps get-and-reassign pattern with setdefault on entry. Apply this to both the junit append path at plugins/ci/skills/payload-snapshot/scripts/payload_snapshot.py lines 1472-1474 and the build_log append path at lines 1488-1490, preserving the existing append behavior.
🤖 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-017-5.0-ci-cccmo-backfilled-junit/annotations.yaml`:
- Around line 24-31: Update the artifact paths in the key_evidence entries to
use the payload tag’s “-backfilled” suffix, matching the input.yaml value.
Preserve the existing job and artifact filenames while replacing the payload
directory segment with the backfilled tag so evaluator matching succeeds.
In `@plugins/ci/skills/payload-snapshot/scripts/payload_snapshot.py`:
- Around line 1472-1474: Update the `summary.json Schema` section of `AGENTS.md`
to document the `data_gaps` field alongside `build_log_errors` and
`test_failure_count`, matching the field introduced in the failed-job handling
near the `state == "Failed"` branch. Keep the existing schema wording and path
documentation unchanged.
---
Nitpick comments:
In `@plugins/ci/skills/payload-snapshot/scripts/payload_snapshot.py`:
- Around line 1472-1474: In the failure-handling blocks of the payload snapshot
logic, replace the redundant data_gaps get-and-reassign pattern with setdefault
on entry. Apply this to both the junit append path at
plugins/ci/skills/payload-snapshot/scripts/payload_snapshot.py lines 1472-1474
and the build_log append path at lines 1488-1490, preserving the existing append
behavior.
🪄 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: cab79bba-09b2-4efb-b71f-e28d39510b5d
📒 Files selected for processing (8)
plugins/ci/evals/cases/payload-analysis/case-015-5.0-nightly-cccmo-false-positive/annotations.yamlplugins/ci/evals/cases/payload-analysis/case-015-5.0-nightly-cccmo-false-positive/input.yamlplugins/ci/evals/cases/payload-analysis/case-016-5.0-ci-cccmo-no-junit/annotations.yamlplugins/ci/evals/cases/payload-analysis/case-016-5.0-ci-cccmo-no-junit/input.yamlplugins/ci/evals/cases/payload-analysis/case-017-5.0-ci-cccmo-backfilled-junit/annotations.yamlplugins/ci/evals/cases/payload-analysis/case-017-5.0-ci-cccmo-backfilled-junit/input.yamlplugins/ci/evals/eval-payload-analysis.yamlplugins/ci/skills/payload-snapshot/scripts/payload_snapshot.py
🚧 Files skipped from review as they are similar to previous changes (3)
- plugins/ci/evals/cases/payload-analysis/case-015-5.0-nightly-cccmo-false-positive/input.yaml
- plugins/ci/evals/eval-payload-analysis.yaml
- plugins/ci/evals/cases/payload-analysis/case-015-5.0-nightly-cccmo-false-positive/annotations.yaml
| artifacts: | ||
| - "5.0/ci/5.0.0-0.ci-2026-07-11-223154/jobs/blocking/aggregated-gcp-ovn-upgrade-5.0-micro/junit/results.json" | ||
| - fact: >- | ||
| "verify operator conditions cloud-controller-manager" passed 2/2 | ||
| with 0 failures. "verify operator conditions machine-config" | ||
| failed 0/2. | ||
| artifacts: | ||
| - "5.0/ci/5.0.0-0.ci-2026-07-11-223154/jobs/blocking/aggregated-gcp-ovn-upgrade-5.0-micro/junit/results.json" |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Align artifact paths with the payload_tag.
The artifact paths in key_evidence are missing the -backfilled suffix. Since the input.yaml for this case specifies payload_tag: "5.0.0-0.ci-2026-07-11-223154-backfilled", the payload directory and the resulting artifact paths cited by the agent will include this suffix. The evaluator may fail to match these strings if they don't exactly align with the agent's output.
🔧 Proposed fix
artifacts:
- - "5.0/ci/5.0.0-0.ci-2026-07-11-223154/jobs/blocking/aggregated-gcp-ovn-upgrade-5.0-micro/junit/results.json"
+ - "5.0/ci/5.0.0-0.ci-2026-07-11-223154-backfilled/jobs/blocking/aggregated-gcp-ovn-upgrade-5.0-micro/junit/results.json"
- fact: >-
"verify operator conditions cloud-controller-manager" passed 2/2
with 0 failures. "verify operator conditions machine-config"
failed 0/2.
artifacts:
- - "5.0/ci/5.0.0-0.ci-2026-07-11-223154/jobs/blocking/aggregated-gcp-ovn-upgrade-5.0-micro/junit/results.json"
+ - "5.0/ci/5.0.0-0.ci-2026-07-11-223154-backfilled/jobs/blocking/aggregated-gcp-ovn-upgrade-5.0-micro/junit/results.json"📝 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.
| artifacts: | |
| - "5.0/ci/5.0.0-0.ci-2026-07-11-223154/jobs/blocking/aggregated-gcp-ovn-upgrade-5.0-micro/junit/results.json" | |
| - fact: >- | |
| "verify operator conditions cloud-controller-manager" passed 2/2 | |
| with 0 failures. "verify operator conditions machine-config" | |
| failed 0/2. | |
| artifacts: | |
| - "5.0/ci/5.0.0-0.ci-2026-07-11-223154/jobs/blocking/aggregated-gcp-ovn-upgrade-5.0-micro/junit/results.json" | |
| artifacts: | |
| - "5.0/ci/5.0.0-0.ci-2026-07-11-223154-backfilled/jobs/blocking/aggregated-gcp-ovn-upgrade-5.0-micro/junit/results.json" | |
| - fact: >- | |
| "verify operator conditions cloud-controller-manager" passed 2/2 | |
| with 0 failures. "verify operator conditions machine-config" | |
| failed 0/2. | |
| artifacts: | |
| - "5.0/ci/5.0.0-0.ci-2026-07-11-223154-backfilled/jobs/blocking/aggregated-gcp-ovn-upgrade-5.0-micro/junit/results.json" |
🤖 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-017-5.0-ci-cccmo-backfilled-junit/annotations.yaml`
around lines 24 - 31, Update the artifact paths in the key_evidence entries to
use the payload tag’s “-backfilled” suffix, matching the input.yaml value.
Preserve the existing job and artifact filenames while replacing the payload
directory segment with the backfilled tag so evaluator matching succeeds.
| elif job_data.get("state") == "Failed": | ||
| entry["data_gaps"] = entry.get("data_gaps", []) | ||
| entry["data_gaps"].append("junit") |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Document the new data_gaps field in the schema.
The data_gaps field is introduced here for failed jobs, but it is not documented in the summary.json Schema section of AGENTS.md. The payload analysis agent relies on this schema to understand the available data, so omitting it could prevent the agent from utilizing the new missing-artifact context.
📝 Proposed fix for AGENTS.md schema
Update the schema documentation (around line 1388) to include data_gaps:
- " `failure_pattern`), `build_log_errors`, `test_failure_count`,",
- " and relative paths to `job_json`, `junit_results`, `build_log`",
+ " `failure_pattern`), `build_log_errors`, `test_failure_count`, `data_gaps`,",
+ " and relative paths to `job_json`, `junit_results`, `build_log`",🤖 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/skills/payload-snapshot/scripts/payload_snapshot.py` around lines
1472 - 1474, Update the `summary.json Schema` section of `AGENTS.md` to document
the `data_gaps` field alongside `build_log_errors` and `test_failure_count`,
matching the field introduced in the failed-job handling near the `state ==
"Failed"` branch. Keep the existing schema wording and path documentation
unchanged.
|
Heads-up from the payload-analysis A/B work in #659/#660: |
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. |
2 participants
Summary
Adds 3 eval cases for payload-analysis targeting the CCCMO #488 false positive, plus judge improvements to catch hallucinated evidence and temporal-correlation-only attribution.
Eval cases
5.0.0-0.nightly-2026-07-13-041249) — Rejected nightly with full JUnit data. MCO root cause is obvious from test results. Tests correct attribution when evidence is available.5.0.0-0.ci-2026-07-11-223154) — Original CI payload as the agent saw it: empty JUnit, no build logs. Tests whether the agent fabricates evidence when data is missing. This is the exact data condition that produced the false positive.5.0.0-0.ci-2026-07-11-223154-backfilled) — Same CI payload with JUnit backfilled from GCS. Tests correct attribution with the data that should have been there originally.Judge improvements
analysis_quality: added hard rules — cap at 2 if report concludes amust_not_concludeverdict or cites fabricated error strings. Score definitions now referencedistractorsanddiscriminating_signal.revert_scoring_accuracy: added hard rules for false-positive cases — cap at 1 ifexpected_candidatesis empty and amust_not_concludePR is recommended for revert. Score definitions now cover no-candidate cases.key_evidence,discriminating_signal,distractors,must_not_conclude(borrowed from prow-job-analysis eval).Snapshot script fix
payload_snapshot.py: failed blocking jobs missing JUnit or build logs now get adata_gapsfield insummary.json(e.g.data_gaps: ["junit", "build_log"]) instead of silently omitting the data.Snapshot data
Depends on stbenjam/historical-payload-data#2 for the 3 payload snapshots.
Context
The payload agent attributed GCP micro upgrade failures to CCCMO PR #488 at 100% confidence (revert PR #489, closed without merging). It fabricated the error string "Deployment cloud-controller-manager rollout not complete" — a klog.V(2) debug line from the PR diff that was never emitted. The actual root cause was MCO: "Operator upgrade machine-config" failed 10/10, while "verify operator conditions cloud-controller-manager" passed 2/2.
🤖 Generated with Claude Code