TRT-2612: feat(ci): teach payload-analysis to read RPM changelogs as evidence - #688
TRT-2612: feat(ci): teach payload-analysis to read RPM changelogs as evidence#688petr-muller wants to merge 3 commits into
Conversation
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: petr-muller 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
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (5)
🚧 Files skipped from review as they are similar to previous changes (3)
WalkthroughThe CI payload-analysis workflow now scores RHCOS RPM changes with changelog evidence. Typed RPM candidates appear with PR candidates in YAML and HTML output, while experiment and revert actions remain PR-only. CI plugin metadata is updated to version 0.0.83. ChangesPayload candidate workflow
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant PayloadAnalysis
participant ChangelogData
participant CandidateResults
participant HTMLReport
PayloadAnalysis->>ChangelogData: Match RPM versions and changelogs
ChangelogData-->>PayloadAnalysis: Return changelog evidence
PayloadAnalysis->>CandidateResults: Add scored rhcos_rpm candidate
CandidateResults->>HTMLReport: Render candidate and changelog data
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 8 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (8 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
/hold |
openshift-ci
Bot
added
the
do-not-merge/hold
|
/test ? |
|
/test eval-payload-analysis-changed |
![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
🤖 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/skills/payload-analysis/SKILL.md`:
- Line 261: Revise the “Subpackage deduplication” guidance to require an
explicit source-RPM/source-package identifier before merging candidates. Do not
infer shared SRPM identity from matching version bumps or changelog text; when
no source key exists in diff.changed[], retain each binary package as a separate
logical candidate.
- Around line 604-638: Update the RHCOS Changes rendering to include RPM
changelog evidence for every relevant originating payload and variant, rather
than using only one originating_payload_tag. In the full changelog diff section,
render all distinct origin-and-variant pairs, including the baseline and each
relevant intermediate hop, while preserving the existing candidate and
collapsible evidence structure.
- Around line 604-638: Update the report-generation section around the RHCOS
Changes card to escape every dynamic value before HTML insertion, including
package, versions, variants, scores, jobs, rationale, changelog text,
snapshot/PR/analysis values, and URLs. Use the existing auto-escaping mechanism
where available, escape text and attribute contexts separately, and permit only
safe URL schemes; do not rely on <pre> to prevent markup injection.
- Around line 249-263: Update Step 3.7b and its subagent instructions to use
each failure mode’s test_failures.blocking[].first_failed_in value for RPM
lookups instead of the job-level originating_payload. For every failure mode,
pass that per-failure first_failed_in payload through when selecting
rhcos_changes[], rpm_changelogs[], and per-hop report files, preserving separate
attribution for jobs with multiple failure modes.
- Around line 450-456: The candidate schema is inconsistent between RPM output,
validation, and reviewer prompts. Update the validator and reviewer input
together so candidates use one score field name, define required fields by
`type` (`pr` versus `rhcos_rpm`), remove obsolete `pr_url` and `rhcos_suspects`
requirements, and replace PR-shaped tuple guidance with the unified typed
candidate schema; update the related sections around the RPM fields,
completeness prompt, and downstream validation references.
🪄 Autofix
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: ba0bce83-248a-48d1-8107-450b2b9c32d5
📒 Files selected for processing (4)
.claude-plugin/marketplace.jsondocs/index.htmlplugins/ci/.claude-plugin/plugin.jsonplugins/ci/skills/payload-analysis/SKILL.md
petr-muller
changed the title
openshift-ci-robot
added
the
jira/valid-reference
|
@petr-muller: This pull request references TRT-2612 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. |
|
/test ? |
|
/test eval-payload-analysis |
petr-muller-reviewer
left a comment
petr-muller-reviewer
left a comment
There was a problem hiding this comment.
This teaches payload-analysis to score RHCOS RPM changes into candidates[] (tagged type: "pr"/type: "rhcos_rpm") instead of a separate rhcos_suspects[] array. The prompt-spec change itself is clearly written and internally consistent, but it changes a schema owned by another skill (payload-results-yaml) without updating that skill's spec/validator, and without updating the one downstream consumer that filters candidates for reverts (payload-revert.md). See the inline comment on SKILL.md for the core issue; the other two files below aren't touched by this PR's diff so I can't anchor comments on them directly:
[blocking] plugins/ci/commands/payload-revert.md:44 — payload-analysis (SKILL.md:442) says RHCOS RPM candidates must never be proposed as reverts and relies on type: "rhcos_rpm" so "downstream tooling can route it correctly." But this command's filter step (Select candidates with confidence_score >= 85) only checks score, with no type check (and no type field exists in the schema yet, per the inline comment). Effect: an RHCOS RPM candidate scoring >=85 (explicitly allowed by this PR's rubric) has no pr_url; stage-payload-reverts templates {pr_url} into a JIRA bug/revert PR, so a later /ci:payload-revert run would attempt a broken revert for a nonexistent PR instead of escalating to the RHCOS/platform team.
[nit] plugins/ci/skills/prow-job-analysis/references/operating-system-changes.md:115 — still says "Surface them as RHCOS RPM suspects," the terminology this PR replaced in payload-analysis (now scored "RHCOS RPM candidates"). This doc cross-references payload-analysis directly, so the mismatched vocabulary reads as if RPM changes are informational-only here, contradicting the new scored-candidate behavior.
Checked and fine with:
- Rubric text in SKILL.md (6.1b RHCOS RPM Candidate Notes) is internally consistent and clearly written.
- Subpackage dedup and variant-isolation guidance make sense given how RHCOS RPM changelogs are structured.
- Version bump present in plugin.json, as expected for a SKILL.md change.
Open questions:
- Is a companion PR planned for
payload-results-yaml(schema/validator/testdata), or should that land in this same PR before merge? - Should
payload-revert.md's filter step be updated now (e.g. atype != "pr"exclusion, or treating missingpr_urlas non-revertable) even before the schema formally addstype? - Any objection to also updating the
operating-system-changes.mdwording to match the new "RHCOS RPM candidate" terminology?
|
@petr-muller-reviewer: changing LGTM is restricted to collaborators 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 kubernetes-sigs/prow repository. |
The payload snapshot now includes RPM changelog diffs (what actually changed in each RPM bump, not just version numbers). Update the payload-analysis skill to consume this data: document the data structures (Step 3.7b), instruct subagents to read changelogs when investigating RHCOS RPM correlations (Step 4), use changelog text to sharpen correlation confidence in Step 6.1b, surface changelog evidence in the HTML report (Step 7.3b), and check for unused changelog evidence in the completeness review (Step 9). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Instead of treating RHCOS RPM changes as a separate "correlation" sidecar that runs after PR scoring (Step 6.1b), integrate them into the main candidate scoring flow. RPM changelogs serve as the RHCOS equivalent of PR diffs — they are now read and matched against failure symptoms using the same rubric. - Step 3.7b: frame changelogs as "the RHCOS equivalent of PR diffs" - Step 4: merge two RHCOS subagent prompt paragraphs into one - Step 6.1: expand candidate universe to include RPM changes alongside PRs; generalize rubric signal descriptions to cover both types - Step 6.1b: rename to "RHCOS RPM Candidate Notes" — no longer a separate scoring pass, just notes on how RPM candidates differ (not revertable, subpackage dedup, hop pinpointing, variant isolation) - RPM candidates live in candidates[] with type: "rhcos_rpm" - Step 6.2: exclude RPM candidates from revert proposals - Step 7.3b: update HTML from "Suspected" to scored "Candidates" - Step 9: merge two reviewer checks into one Review follow-ups folded in: - RHCOS RPM lookups (Step 3.7, Step 4 prompt) now use the failure mode's first_failed_in instead of the job-level streak.originating_payload, matching how candidate PRs are already re-derived per failure mode - Name the RPM candidate score field confidence_score, matching PR candidates, instead of the ambiguous "score" - Step 7.3b HTML renders the baseline RPM changelog diff plus a subsection per relevant intermediate hop, instead of assuming every failure mode shares one origin Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
bbd8ac9 to
b845b27
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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/skills/payload-results-yaml/scripts/validate.py`:
- Around line 9-13: Align candidate validation with the documented schema: in
plugins/ci/skills/payload-results-yaml/scripts/validate.py lines 9-13, update
REQUIRED_CANDIDATE_FIELDS to require rationale and actions and update
REQUIRED_CANDIDATE_FIELDS_BY_TYPE for the documented type-specific fields. In
plugins/ci/skills/payload-results-yaml/scripts/testdata/valid.yaml lines 19-20,
add rationale to the PR candidate. In
plugins/ci/skills/payload-results-yaml/scripts/testdata/valid_with_rhcos_rpm_candidate.yaml
lines 18-25, add actions as an empty list and add changelog_evidence to the RPM
candidate.
- Around line 66-78: Update the candidate validation logic around
REQUIRED_CANDIDATE_FIELDS_BY_TYPE and cand_type to reject non-string type values
before dictionary membership lookup, append an input-validation error, and
ensure validation returns 1 without raising for YAML values such as lists or
mappings. Add a regression fixture covering these cases.
In `@plugins/ci/skills/payload-results-yaml/SKILL.md`:
- Around line 128-162: Update the payload-results schema and processing
operations to handle legacy files before reading candidates: add and enforce a
schema version that rejects older files, or migrate legacy PR and rhcos_suspects
data into typed candidates[] entries with the required type fields. Ensure
migration or rejection occurs before filtering, staging, or any other operation,
and preserve current typed-candidate behavior.
🪄 Autofix
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: 3c99a6e0-09ea-46db-a013-d126651cd08c
📒 Files selected for processing (10)
plugins/ci/commands/payload-experiment.mdplugins/ci/commands/payload-revert.mdplugins/ci/skills/payload-analysis/SKILL.mdplugins/ci/skills/payload-results-yaml/SKILL.mdplugins/ci/skills/payload-results-yaml/scripts/test_validate.pyplugins/ci/skills/payload-results-yaml/scripts/testdata/invalid_rhcos_rpm_candidate.yamlplugins/ci/skills/payload-results-yaml/scripts/testdata/valid.yamlplugins/ci/skills/payload-results-yaml/scripts/testdata/valid_with_rhcos_rpm_candidate.yamlplugins/ci/skills/payload-results-yaml/scripts/validate.pyplugins/ci/skills/prow-job-analysis/references/operating-system-changes.md
…load-analysis payload-analysis now writes RHCOS RPM changes into candidates[] with type: "rhcos_rpm" alongside PR candidates (type: "pr"), but the schema they write against, and the two commands that read it, still assumed the old shape: a separate rhcos_suspects[] array, no type field, and a validator that unconditionally required pr_url on every candidate. Any payload-results YAML written under the new instructions failed validation, and a high-confidence RHCOS RPM candidate had no PR to revert. - payload-results-yaml: fold rhcos_suspects[] into candidates[], document common vs type-specific fields, require type-conditional fields in validate.py (pr_url for "pr"; package/rhcos_tag/ changelog_evidence for "rhcos_rpm"; rationale/actions for both), guard against non-string type values instead of crashing on unhashable dict membership, update test fixtures accordingly - payload-revert.md, payload-experiment.md: filter to type: "pr" candidates explicitly, excluding rhcos_rpm candidates from reverts and experiments (they cannot be pulled out of a payload like a PR) - operating-system-changes.md: update stale "RHCOS RPM suspects" terminology to match the new scored-candidate flow Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
b845b27 to
0b0d666
Compare
|
/test eval-payload-analysis |
|
/hold cancel |
openshift-ci
Bot
removed
the
do-not-merge/hold
3 participants
The payload snapshot now includes RPM changelog diffs (what actually changed in each RPM bump, not just version numbers). Update the payload-analysis skill to consume this data: document the data structures, instruct subagents to read changelogs when investigating RHCOS RPM correlations, use changelog text to sharpen correlation confidence in, surface changelog evidence in the HTML report, and check for unused changelog evidence in the completeness review.
Instead of treating RHCOS RPM changes as a separate "correlation" sidecar that runs after PR scoring, integrate them into the main candidate scoring flow. RPM changelogs serve as the RHCOS equivalent of PR diffs — they are now read and matched against failure symptoms using the same rubric.
Summary by CodeRabbit
New Features
Bug Fixes
Chores