Improve payload analysis depth and coverage - #356
openshift-merge-bot[bot] merged 4 commits into
Conversation
|
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:
WalkthroughCI plugin version bumped from 0.0.17 → 0.0.19 in manifests and docs; prow install-failure guidance rewritten into a log-driven diagnostic procedure with TechPreview notes; analyze-payload docs and skill expanded to include Accepted payloads, broaden lookback semantics, and add failure_pattern reporting and reporting/UI enhancements. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 8✅ Passed checks (8 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@plugins/ci/skills/prow-job-analyze-install-failure/SKILL.md`:
- Around line 322-323: The sentence "A crash is a software bug until proven
otherwise." in SKILL.md (the bullet about reading stderr/stdout for crashed
processes) is too absolute—update that line to require validating termination
reason before labeling a crash as a product bug; replace the sentence with a
phrasing such as "Treat a crash as a potential bug but validate the termination
reason (OOM, host restart, killed by signal, resource limits, etc.) by examining
container exit status, kernel messages, and host metrics before assigning root
cause." Also add a short note to consult surrounding logs and infra signals
(exit codes, ContainerDied event details) to distinguish software defects from
infra/resource-induced terminations.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 0dd6c0a7-8bb1-4ea0-b94f-ecca4e129e71
📒 Files selected for processing (2)
plugins/ci/.claude-plugin/plugin.jsonplugins/ci/skills/prow-job-analyze-install-failure/SKILL.md
1bdf62b to
28f5e4e
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
plugins/ci/skills/analyze-payload/SKILL.md (1)
71-71: Tighten redundant phrasing for readability.“first started failing” is repetitive; “started failing” is cleaner in both places.
✏️ Suggested wording tweak
-The goal is to determine **when each failing job first started failing** and understand its failure pattern across recent payloads. +The goal is to determine **when each failing job started failing** and understand its failure pattern across recent payloads. ... -- **is_new_failure**: Whether the job first started failing in the target payload (streak_length == 1) +- **is_new_failure**: Whether the job started failing in the target payload (streak_length == 1)Also applies to: 82-82
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/ci/skills/analyze-payload/SKILL.md` at line 71, Replace the redundant phrase "first started failing" with the shorter "started failing" in the analyze-payload skill description (SKILL.md); locate the occurrences of the exact phrase "first started failing" (including the goal sentence and the second instance later in the file) and update both to "started failing" to tighten the wording while preserving the original meaning.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@plugins/ci/skills/analyze-payload/SKILL.md`:
- Line 71: Replace the redundant phrase "first started failing" with the shorter
"started failing" in the analyze-payload skill description (SKILL.md); locate
the occurrences of the exact phrase "first started failing" (including the goal
sentence and the second instance later in the file) and update both to "started
failing" to tighten the wording while preserving the original meaning.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 64dc9c0e-4556-496c-b399-40ffffb4589f
📒 Files selected for processing (2)
plugins/ci/commands/analyze-payload.mdplugins/ci/skills/analyze-payload/SKILL.md
There was a problem hiding this comment.
🧹 Nitpick comments (1)
PLUGINS.md (1)
61-61: Use canonical payload phase terminology for consistency.Line 61 says “in-progress,” while the PR objective references phase names including “Ready.” Consider aligning this wording with the canonical phase labels used elsewhere to avoid ambiguity in operator guidance.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@PLUGINS.md` at line 61, The description for the /ci:analyze-payload command uses “in-progress” which is inconsistent with canonical payload phase terminology; update the text for **`/ci:analyze-payload` `<payload-tag> [--lookback N]`** to use the official phase label (e.g., replace “in-progress” with “Ready” or the exact phase name used elsewhere), ensure capitalization and punctuation match other docs, and keep the list order and phrasing consistent with existing references to “Rejected, Accepted, Ready” (or the project's canonical phase names).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@PLUGINS.md`:
- Line 61: The description for the /ci:analyze-payload command uses
“in-progress” which is inconsistent with canonical payload phase terminology;
update the text for **`/ci:analyze-payload` `<payload-tag> [--lookback N]`** to
use the official phase label (e.g., replace “in-progress” with “Ready” or the
exact phase name used elsewhere), ensure capitalization and punctuation match
other docs, and keep the list order and phrasing consistent with existing
references to “Rejected, Accepted, Ready” (or the project's canonical phase
names).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 8f5c7570-3d0c-429f-9a62-f400fc6054be
📒 Files selected for processing (2)
PLUGINS.mddocs/data.json
🚧 Files skipped from review as they are similar to previous changes (1)
- docs/data.json
| "source": "./plugins/ci", | ||
| "description": "A plugin to work with OpenShift CI and analyze Prow job results", | ||
| "version": "0.0.17" | ||
| "version": "0.0.18" |
There was a problem hiding this comment.
We're about to conflict, sorry! :(
078d5c3 to
0cc689e
Compare
Bootstrap failures are varied and complex. The previous guidance was too
thin ("check for bootkube errors") and led to misdiagnoses when the agent
didn't read actual error output from crashed processes. Replace with
systematic 4-step guidance: build a full timeline from bootkube.log,
pursue every error by reading context and following causation chains,
cross-reference supporting logs, and check OS-level problems. Also add
TechPreview job awareness and a general process crash pattern.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…orough investigation - Analyze accepted payloads instead of skipping them (force-accepts may have failed blocking jobs) - Scan entire lookback window regardless of payload phase, tracking full pass/fail pattern (F F S F F) instead of stopping at first pass - Add failure_pattern field to history, HTML report, and JSON schema - Require full analysis (no --fast) for test failure subagents - Always perform thorough investigation including must-gather extraction Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
0cc689e to
bf330c0
Compare
|
/lgtm |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
plugins/ci/skills/analyze-payload/SKILL.md (1)
71-71: Trim redundant wording for readability.Line 71 and Line 82 use “first started failing”; “started failing” is cleaner.
Also applies to: 82-82
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/ci/skills/analyze-payload/SKILL.md` at line 71, In SKILL.md update the wording to remove redundancy by replacing the phrase "first started failing" with the shorter "started failing" in both occurrences referenced (the sentence containing "when each failing job first started failing" and the similar sentence at the other occurrence), ensuring the intent remains the same and spacing/punctuation remains consistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@plugins/ci/skills/analyze-payload/SKILL.md`:
- Line 650: The documentation uses two delimiter styles for the failure_pattern
(e.g., "F-F-F-S-F-F-F" vs "F F F S F F"); choose one canonical format (recommend
space-delimited) and update all occurrences in SKILL.md to that single format,
including the example sentence currently showing "F-F-F-S-F-F-F" and any earlier
examples that use "F F F S F F"; ensure the README’s description of
failure_pattern and any example headings or inline examples consistently
reference the chosen delimiter so downstream parsers/readers see one canonical
representation.
---
Nitpick comments:
In `@plugins/ci/skills/analyze-payload/SKILL.md`:
- Line 71: In SKILL.md update the wording to remove redundancy by replacing the
phrase "first started failing" with the shorter "started failing" in both
occurrences referenced (the sentence containing "when each failing job first
started failing" and the similar sentence at the other occurrence), ensuring the
intent remains the same and spacing/punctuation remains consistent.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: ce16c26e-e8a5-4707-8a03-320df9a6a23d
📒 Files selected for processing (7)
.claude-plugin/marketplace.jsonPLUGINS.mddocs/data.jsonplugins/ci/.claude-plugin/plugin.jsonplugins/ci/commands/analyze-payload.mdplugins/ci/skills/analyze-payload/SKILL.mdplugins/ci/skills/prow-job-analyze-install-failure/SKILL.md
✅ Files skipped from review due to trivial changes (1)
- PLUGINS.md
🚧 Files skipped from review as they are similar to previous changes (3)
- plugins/ci/commands/analyze-payload.md
- .claude-plugin/marketplace.json
- docs/data.json
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: dgoodwin, 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 |
Summary
Improvements to the CI payload analysis and install failure investigation skills to produce more thorough and accurate results.
Payload analysis (
analyze-payload)failure_patternfield records the full pass/fail history across the lookback window, shown in the HTML report and JSON output.Install failure analysis (
prow-job-analyze-install-failure)🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation
Chores