Skip to content

USHIFT-7419: CI Doctor: --prepared argument - #253

Open
pmtk wants to merge 2 commits into
openshift-eng:mainfrom
pmtk:ci-doctor-prepared
Open

USHIFT-7419: CI Doctor: --prepared argument#253
pmtk wants to merge 2 commits into
openshift-eng:mainfrom
pmtk:ci-doctor-prepared

Conversation

@pmtk

@pmtk pmtk commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

--prepared means that the artifacts were already prefetch and there is no need to waste tokens on running deterministic commands

Summary by CodeRabbit

  • New Features

    • Added optional --prepared support to doctor workflows.
    • Workflows can now use an existing preparation summary and skip redundant artifact preparation.
    • MicroShift workflows also skip graph generation when prepared artifacts are supplied.
  • Documentation

    • Clarified workdir determination, summary parsing, and conditional workflow steps.

@openshift-ci

openshift-ci Bot commented Aug 3, 2026

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: pmtk

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci openshift-ci Bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 3, 2026
@coderabbitai

coderabbitai Bot commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

@pmtk, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 52 minutes

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Repository YAML (base), Central YAML (inherited)

Review profile: CHILL

Plan: Pro Plus

Run ID: 15ccb6fa-8b53-4d86-94af-d455176551b0

📥 Commits

Reviewing files that changed from the base of the PR and between 1b2f2a0 and 42baf52.

📒 Files selected for processing (8)
  • .claude-plugin/marketplace.json
  • plugins/lvms-ci/.claude-plugin/plugin.json
  • plugins/lvms-ci/hooks/hooks.json
  • plugins/lvms-ci/skills/doctor/SKILL.md
  • plugins/microshift-ci/.claude-plugin/plugin.json
  • plugins/microshift-ci/.claude/settings.json
  • plugins/microshift-ci/hooks/hooks.json
  • plugins/microshift-ci/skills/doctor/SKILL.md

Walkthrough

The doctor skills now accept --prepared to read an existing prepare-summary.json. Prepared MicroShift runs also skip PCP graph generation. Default runs retain the existing preparation flow.

Changes

Prepared doctor workflow

Layer / File(s) Summary
Prepared summary handling
plugins/lvms-ci/skills/doctor/SKILL.md, plugins/microshift-ci/skills/doctor/SKILL.md
Both skills document --prepared and read the existing preparation summary. Without the flag, they run the preparation script as before.
MicroShift graph-generation branch
plugins/microshift-ci/skills/doctor/SKILL.md
Prepared runs skip PCP graph generation and continue to analysis. Non-prepared runs retain graph generation.

Estimated code review effort: 2 (Simple) | ~10 minutes

Suggested labels: ready-for-human-review

Suggested reviewers: kasturinarra, ggiguash

🚥 Pre-merge checks | ✅ 11
✅ Passed checks (11 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly identifies the CI Doctor change and the addition of the --prepared argument.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
No-Weak-Crypto ✅ Passed The pull request changes only two Markdown skill files for --prepared; added lines contain no weak-crypto algorithms, custom crypto, or secret comparisons.
Container-Privileges ✅ Passed The PR changes only two SKILL.md files. The added documentation contains no privileged, hostPID, hostNetwork, hostIPC, SYS_ADMIN, or allowPrivilegeEscalation settings.
No-Sensitive-Data-In-Logs ✅ Passed The PR changes only doctor documentation; added lines contain no credentials, tokens, PII, hostnames, or logging statements.
No-Hardcoded-Secrets ✅ Passed The PR changes only two SKILL.md files; added lines contain no sensitive identifiers, credential URLs, secret assignments, or base64-like strings over 32 characters.
No-Injection-Vectors ✅ Passed The PR changes only two SKILL.md documentation files; added lines contain no SQL, eval/exec, pickle.loads, unsafe YAML, os.system, shell=True, or dangerouslySetInnerHTML.
Ai-Attribution ✅ Passed The PR description and HEAD commit do not mention AI tool use, and HEAD has no Co-Authored-By, Assisted-by, or Generated-by trailer.
✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot added the ready-for-human-review Indicates a PR has been reviewed by automated tools and is ready for human review label Aug 3, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 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/lvms-ci/skills/doctor/SKILL.md`:
- Around line 41-43: Update the --prepared branches in
plugins/lvms-ci/skills/doctor/SKILL.md (lines 41-43) and
plugins/microshift-ci/skills/doctor/SKILL.md (lines 40-42) to validate
prepare-summary.json immediately after reading and parsing it, including JSON
validity, required fields, workdir, and the branch-specific release/PR data
(LVMS: release info and PR info; MicroShift: source paths plus release/PR info).
Fail closed with the shared error handling before proceeding to Step 2, keep
each failure policy beside its read/parse action, and add applicable positive
and negative coverage per CONTRIBUTING.md.

In `@plugins/microshift-ci/skills/doctor/SKILL.md`:
- Around line 75-78: Update the prepared-mode flow in the doctor skill to
validate <WORKDIR>/graphs/<build_id> for every job with PCP data before
launching agents or proceeding to Step 2. Detect missing graph output using the
contract in the PCP graph-generation documentation, and stop or clearly report
each affected job instead of allowing analysis or finalization to continue
without graphs; document this guard and failure behavior between the phases.
🪄 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: Repository YAML (base), Central YAML (inherited)

Review profile: CHILL

Plan: Pro Plus

Run ID: 4ef046d1-77e9-4855-b122-ed6d395eb78d

📥 Commits

Reviewing files that changed from the base of the PR and between a0f0c9b and 1b2f2a0.

📒 Files selected for processing (2)
  • plugins/lvms-ci/skills/doctor/SKILL.md
  • plugins/microshift-ci/skills/doctor/SKILL.md

Comment thread plugins/lvms-ci/skills/doctor/SKILL.md
Comment on lines +75 to +78
**If `--prepared` was passed**: skip this step entirely (graphs were already generated externally). Proceed to Step 2.

**Otherwise**:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift

Validate pre-generated PCP graphs before skipping Step 1b.

Prepared mode trusts the external graph generation without checking <WORKDIR>/graphs/<build_id>. Later, Step 2 omits graphs_dir when the directory is absent. The analyzer can therefore run without graphs, and finalization cannot embed graph files that are missing.

Check the expected graph directory for each job with PCP data before launching agents. Stop or report the affected jobs when graph output is missing.

Per plugins/docs/SKILL-GUIDELINES.md, document failure cases and guard checks between phases. plugins/microshift-ci/scripts/pcp-graphs/README.md:61-69 defines the graph output contract.

🤖 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/microshift-ci/skills/doctor/SKILL.md` around lines 75 - 78, Update
the prepared-mode flow in the doctor skill to validate
<WORKDIR>/graphs/<build_id> for every job with PCP data before launching agents
or proceeding to Step 2. Detect missing graph output using the contract in the
PCP graph-generation documentation, and stop or clearly report each affected job
instead of allowing analysis or finalization to continue without graphs;
document this guard and failure behavior between the phases.

Source: Path instructions

@pmtk
pmtk force-pushed the ci-doctor-prepared branch from 1b2f2a0 to 1d7447b Compare August 3, 2026 12:22
pmtk added 2 commits August 3, 2026 16:01
--prepared means that the artifacts were already prefetch and there is
no need to waste tokens on running deterministic commands
@pmtk
pmtk force-pushed the ci-doctor-prepared branch from f3efe75 to 42baf52 Compare August 3, 2026 14:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. ready-for-human-review Indicates a PR has been reviewed by automated tools and is ready for human review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant