ci: remove analyze-payload and add RHCOS version to payload snapshots - #519
Conversation
Remove the legacy analyze-payload command and skill, superseded by payload-analysis. Port the RHCOS version determination logic into the payload_snapshot.py script so each job in summary.json includes an rhcos_version field (rhcos9, rhcos10, rhcos9_10, or rhcos9-default). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
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 (2)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughCI plugin version bumped to 0.0.45; GEMINI example command updated. Payload-snapshot adds per-job ChangesCI Plugin Version Bump and RHCOS Version Tracking
sequenceDiagram
participant Snapshotter
participant _extract_jobs
participant JobCollector_fetch
participant SummaryGenerator
Snapshotter->>_extract_jobs: pass OCP version (self.tag.version)
_extract_jobs->>_extract_jobs: compute rhcos_version via _determine_rhcos_version
_extract_jobs->>JobCollector_fetch: return JobInfo with rhcos_version
JobCollector_fetch->>SummaryGenerator: write job.json (includes rhcos_version)
SummaryGenerator->>SummaryGenerator: include rhcos_version in failed-job entries and report rendering
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 9 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (9 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
/cc |
Update payload-analysis skill to read rhcos_version from snapshot data, pass it to subagents, perform variant isolation analysis, and render RHCOS badges in the HTML report. Document the new rhcos_version field in the payload-snapshot SKILL.md. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
plugins/ci/skills/payload-analysis/SKILL.md (1)
1-535:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRun the required 4-phase overlap check for
plugins/ci/skills/payload-analysis/SKILL.md(include HIGH/MODERATE/LOW).The earlier overlap check output wasn’t provided in the review, and the prior search indicates there is at least one concurrent open PR editing the same skill:
#519(ci: remove analyze-payload and add RHCOS version to payload snapshots), with changes includingplugins/ci/skills/payload-analysis/SKILL.md. The 4-phase process (filter → quick title/path → semantic comparison w/ ≥60% threshold → HIGH/MODERATE/LOW) should be run against the relevant PR(s) and summarized before merge.🤖 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-analysis/SKILL.md` around lines 1 - 535, The reviewer requests a 4-phase overlap check for plugins/ci/skills/payload-analysis/SKILL.md and a summary of overlap severity (HIGH/MODERATE/LOW) before merging; run the canonical 4-phase process (filter by concurrent PRs editing same file, quick title/path similarity, semantic comparison with ≥60% threshold, then classify conflict severity) specifically against open PR `#519` (and any other PRs that touch SKILL.md), produce the overlap results (which PRs passed each phase, the semantic similarity score(s), and a final HIGH/MODERATE/LOW verdict per PR), and include that summary as a review comment prior to merge so maintainers can decide how to proceed.
🤖 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`:
- Around line 295-296: The documentation currently lists CSS badge classes
badge-rhcos9, badge-rhcos10 and badge-rhcos-mixed but the schema also accepts
rhcos9-default; add an explicit mapping that treats rhcos9-default the same as
rhcos9 by documenting that rhcos9-default renders with the badge-rhcos9 visual
style (and note adding the variant-isolated class if applicable). Update every
place that enumerates RHCOS badge examples/styles (references to badge-rhcos9,
badge-rhcos10, badge-rhcos-mixed and the rhcos entries) so rhcos9-default is
shown and described as using badge-rhcos9 (ensure the same wording appears in
the sections currently listing rhcos badges).
- Around line 170-174: The documentation for RHCOS variant isolation omits how
`rhcos9-default` should be treated; update the "RHCOS variant isolation" section
to state that `rhcos9-default` counts as RHCOS 9 for isolation checks (i.e., it
contributes to the RHCOS 9 side when computing `failure_scope` values like
"rhcos9-only" and "rhcos10-only"), alongside the existing note that `rhcos9_10`
counts toward both variants; explicitly mention `rhcos9-default` by name so
implementations of `failure_scope` logic treat it as RHCOS 9.
In `@plugins/ci/skills/payload-snapshot/SKILL.md`:
- Around line 192-196: The documentation claims `rhcos9-default` is derived from
the OCP major version but the implementation always returns `rhcos9-default` for
the no-fragment path; update the SKILL.md text that lists `rhcos_version` (the
paragraph describing `rhcos9-default`) to accurately reflect current behavior by
softening the wording (e.g., “currently defaults to `rhcos9-default` when no
fragment is present” or similar) so the docs match the actual logic rather than
implying OCP-major-based derivation.
- Line 161: Update the SKILL.md description for blocking_jobs.failed_jobs[] to
clearly mark conditional fields as optional: change the sentence listing
rhcos_version, streak, build_log_errors, and test_failure_count to indicate they
"may include" or append "(optional)" to each conditional field, and ensure
streak's subfields (streak_length, originating_payload, is_new_failure,
failure_pattern) are also marked optional; locate the text referencing
blocking_jobs.failed_jobs[] and replace the current definitive list with wording
that matches the generated schema (e.g., "may include: rhcos_version, streak
(may include ...), build_log_errors, test_failure_count, and relative paths
...").
---
Outside diff comments:
In `@plugins/ci/skills/payload-analysis/SKILL.md`:
- Around line 1-535: The reviewer requests a 4-phase overlap check for
plugins/ci/skills/payload-analysis/SKILL.md and a summary of overlap severity
(HIGH/MODERATE/LOW) before merging; run the canonical 4-phase process (filter by
concurrent PRs editing same file, quick title/path similarity, semantic
comparison with ≥60% threshold, then classify conflict severity) specifically
against open PR `#519` (and any other PRs that touch SKILL.md), produce the
overlap results (which PRs passed each phase, the semantic similarity score(s),
and a final HIGH/MODERATE/LOW verdict per PR), and include that summary as a
review comment prior to merge so maintainers can decide how to proceed.
🪄 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: 4df4dcb3-675c-4d25-b8d0-cf33d4d3047a
📒 Files selected for processing (2)
plugins/ci/skills/payload-analysis/SKILL.mdplugins/ci/skills/payload-snapshot/SKILL.md
- Clarify that rhcos9-default counts as RHCOS 9 for variant isolation - Document that rhcos9-default renders with badge-rhcos9 CSS class - Mark conditional fields as optional in summary.json schema docs - Soften rhcos9-default wording to match current implementation Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
/test ? |
|
/test eval-payload-analysis-minimal |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: petr-muller, 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 |
|
/hold prevent accidental merge, unhold as needed |
|
/hold cancel |
|
/hold Ah wait openshift/release#79922 needs to go first |
|
/hold cancel |
Summary
analyze-payloadcommand and skill, superseded bypayload-analysispayload_snapshot.pyso each job insummary.jsonincludes anrhcos_versionfield (rhcos9,rhcos10,rhcos9_10, orrhcos9-default)Test plan
python3 -c "import ast; ast.parse(open('plugins/ci/skills/payload-snapshot/scripts/payload_snapshot.py').read())"passesrhcos_version: rhcos9-defaultrhcos10jobs and confirm correct detection/ci:payload-analysisstill works end-to-end without the removedanalyze-payloadfiles🤖 Generated with Claude Code
Summary by CodeRabbit
Chores
Documentation
New Features