Skip to content

Fix quality checks for README reorders - #6589

Open
matveynator wants to merge 1 commit into
avelino:mainfrom
matveynator:agent/skip-readme-reorders
Open

Fix quality checks for README reorders#6589
matveynator wants to merge 1 commit into
avelino:mainfrom
matveynator:agent/skip-readme-reorders

Conversation

@matveynator

Copy link
Copy Markdown
Contributor

Summary

  • distinguish pure README entry reorders from package additions or updates
  • skip package metadata validation when the added and removed entry sets are identical
  • preserve all existing quality checks for actual package changes

Root cause

The current detector classifies every pull request touching README.md as a package PR. Maintenance-only reorders therefore run package-submission validation against an unchanged entry and can fail link consistency, as seen in #6586.

Validation

The detector logic was evaluated against the live patches for:

The updated workflow also parses successfully as YAML.

@github-actions

Copy link
Copy Markdown

Automated Quality Checks

Skipped — this PR does not modify README.md, so package quality checks do not apply.

This is expected for maintenance, documentation, or workflow PRs.

@matveynator
matveynator marked this pull request as ready for review August 16, 2026 09:53
@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Fix PR quality check detection for README-only package reorders

🐞 Bug fix ⚙️ Configuration changes 🕐 10-20 Minutes

Grey Divider

AI Description

• Detect pure README package-entry reorders and avoid treating them as package submissions.
• Skip package metadata validation when added/removed README entries are identical.
• Preserve existing quality checks for real package additions/updates.
Diagram

graph TD
  A["PR event"] --> B["Detect PR type job"] --> C["GitHub API via gh"] --> D["README.md patch"] --> E["Parse + sort entry lines"] --> F["is_package_pr output"]
  F --> G["Quality checks job (runs if true)"]
  F --> H["Skip notice job (runs if false)"]
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Compare base vs head README content sets
  • ➕ Avoids relying on GitHub's per-file .patch field (which can be empty/truncated)
  • ➕ More robust to diff context/formatting changes
  • ➖ Requires additional API calls (fetch file contents by SHA) or a checkout in the detect job
  • ➖ More implementation complexity than patch parsing
2. Always run quality checks but make them reorder-tolerant
  • ➕ Eliminates classification logic entirely
  • ➕ Ensures consistent enforcement across all README changes
  • ➖ Requires changes to the quality-check tooling, not just workflow gating
  • ➖ Still spends CI time on maintenance-only reorder PRs

Recommendation: The current patch-based gating is a pragmatic, low-cost fix that preserves existing behavior for real package changes. If patch truncation/emptiness becomes an issue, switch to comparing README contents (base vs head) by SHA to make detection robust.

Files changed (1) +13 / -3

Other (1) +13 / -3
pr-quality-check.yamlRefine package PR detection to ignore pure README entry reorders +13/-3

Refine package PR detection to ignore pure README entry reorders

• Replaces the "README modified => package PR" heuristic with patch-based detection. Extracts added/removed package entry lines from the README diff and skips package quality checks when the entry sets are identical, while still running checks for actual additions/updates.

.github/workflows/pr-quality-check.yaml

@qodo-code-review

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0) 📜 Skill insights (0)

Grey Divider


Remediation recommended

1. Non-entry edits bypass validation 🐞 Bug ≡ Correctness
Description
The detector treats any README patch whose added and removed package-entry lines match as a pure
reorder, even when the same patch also changes headings, prose, category structure, or other README
content. Such a PR is marked non-package and skips both quality and diff checks despite modifying
the package catalog file.
Code

.github/workflows/pr-quality-check.yaml[R30-32]

+            if [ -n "$added_entries" ] && [ "$added_entries" = "$removed_entries" ]; then
+              echo "is_package_pr=false" >> "$GITHUB_OUTPUT"
+              echo "README.md only reorders existing package entries — skipping quality checks"
Relevance

●●● Strong

CI/diff validation is important here; bypassing checks for non-entry README edits conflicts with
existing diff checker behavior.

PR-#6055
PR-#6106

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The workflow's only discriminator after finding README.md is equality of extracted entry lines, so
all other patch lines are ignored. The downstream diff checker explicitly handles README category or
formatting changes with no package entries, showing that README modifications outside entry
additions are a supported validation case.

.github/workflows/pr-quality-check.yaml[24-35]
.github/scripts/check-pr-diff/main.go[89-110]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
A README patch is classified as a reorder solely because sorted added and removed package-entry lines are identical. Unrelated README edits in the same patch therefore bypass package and diff validation.

## Issue Context
Only skip checks when the complete README diff represents an entry reorder; headings, prose, category structure, formatting, and other non-entry changes must prevent the skip. The downstream diff checker handles README changes with no package entries and should remain available for those cases.

## Fix Focus Areas
- .github/workflows/pr-quality-check.yaml[25-35]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Context
✅ Compliance rules (platform): 15 rules
Review mode: 🚀 Fast: This is a localized CI workflow change with one edit site and contained behavior, without security, data, API, or other high-risk impact.

Grey Divider

Tip of the day
💡 Did you know, you can add REVIEW.md to your repo root and Qodo follows it on every PR

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

Comment on lines +30 to +32
if [ -n "$added_entries" ] && [ "$added_entries" = "$removed_entries" ]; then
echo "is_package_pr=false" >> "$GITHUB_OUTPUT"
echo "README.md only reorders existing package entries — skipping quality checks"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Remediation recommended

1. Non-entry edits bypass validation 🐞 Bug ≡ Correctness

The detector treats any README patch whose added and removed package-entry lines match as a pure
reorder, even when the same patch also changes headings, prose, category structure, or other README
content. Such a PR is marked non-package and skips both quality and diff checks despite modifying
the package catalog file.
Agent Prompt
## Issue description
A README patch is classified as a reorder solely because sorted added and removed package-entry lines are identical. Unrelated README edits in the same patch therefore bypass package and diff validation.

## Issue Context
Only skip checks when the complete README diff represents an entry reorder; headings, prose, category structure, formatting, and other non-entry changes must prevent the skip. The downstream diff checker handles README changes with no package entries and should remain available for those cases.

## Fix Focus Areas
- .github/workflows/pr-quality-check.yaml[25-35]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

@qodo-code-review

Copy link
Copy Markdown

Qodo Fixer

✅ Merged (0) · ☑ Fixed (0)

Process

  • No fixes were applied (no_fixes_applied)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant