doc-gate: accept changelog.d fragments so concurrent PRs stop conflicting - #2290
Conversation
|
Warning Review limit reached
Next review available in: 35 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 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 configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (7)
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. Comment |
PR Summary by QodoDoc-gate: allow changelog.d fragments + collator to reduce changelog conflicts
AI Description
Diagram
High-Level Assessment
Files changed (7)
|
Code Review by Qodo
1. Fragments deleted without insertion
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
| anchor = UNRELEASED + "\n" | ||
| text = text.replace(anchor, anchor + "\n" + section, 1) | ||
| CHANGELOG.write_text(text, encoding="utf-8") |
There was a problem hiding this comment.
1. Fragments deleted without insertion 🐞 Bug ≡ Correctness
scripts/collate_changelog.py only verifies that "## [Unreleased]" appears somewhere, but inserts by replacing the exact substring "## [Unreleased]\n"; if the Unreleased header line lacks a trailing newline, the replace is a no-op and the script still deletes all fragment files, losing release notes.
Agent Prompt
### Issue description
The collator checks `UNRELEASED in text` but performs insertion via `text.replace(UNRELEASED + "\n", ...)`. If the header is present without a trailing newline, insertion does not occur but fragments are still deleted.
### Issue Context
This is release-time, destructive behavior: a no-op insertion followed by fragment deletion silently drops changelog entries.
### Fix Focus Areas
- scripts/collate_changelog.py[105-116]
### What to change
- Perform line-aware insertion (e.g., find the `## [Unreleased]` line boundary and insert immediately after it), rather than relying on an exact `"...\n"` substring.
- After attempting insertion, assert it succeeded (e.g., compare `text_before`/`text_after` or check a sentinel) and if not, print an error to stderr and `return 1` **before** unlinking any fragments.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| merged, consumed = collect(FRAGMENT_DIR) | ||
| if not consumed: | ||
| print("collate-changelog: no fragments, nothing to do") | ||
| return 0 |
There was a problem hiding this comment.
2. Malformed fragments crash collator 🐞 Bug ☼ Reliability
parse_fragment() raises ValueError for an invalid/empty fragment, but main() does not catch it, so the script exits via an uncaught exception/traceback instead of returning the documented exit code 1 for malformed fragments.
Agent Prompt
### Issue description
`parse_fragment()` can raise `ValueError`, but `main()` doesn’t handle it, so a malformed fragment produces a traceback rather than a clean error + exit code 1.
### Issue Context
This is a release bump tool; malformed contributor input should fail loudly but cleanly, with a stable exit code and no partial side effects.
### Fix Focus Areas
- scripts/collate_changelog.py[55-57]
- scripts/collate_changelog.py[90-99]
### What to change
- Wrap `collect(FRAGMENT_DIR)` (or the body of `collect`) in `try/except ValueError`.
- On error: print a concise message to stderr (include fragment filename), return 1, and ensure fragments are not deleted.
- Optionally add a test asserting malformed fragments return 1 without a traceback.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| for path in sorted(fragment_dir.glob("*.md")): | ||
| for section, lines in parse_fragment(path).items(): | ||
| merged.setdefault(section, []).extend(lines) | ||
| consumed.append(path) |
There was a problem hiding this comment.
3. Deletes unexpected markdown files 🐞 Bug ☼ Reliability
collect() consumes every top-level changelog.d/*.md file and main() deletes all consumed paths, so any accidental non-fragment markdown file (e.g., a future template/README) would be folded into CHANGELOG.md and then removed.
Agent Prompt
### Issue description
The collator globs `changelog.d/*.md` and deletes everything it consumes. If a non-fragment `.md` ends up in that directory, it will be incorporated into the changelog and then deleted.
### Issue Context
The convention doc specifies a `<pr-number>-<short-slug>.md` naming scheme, but the script currently doesn’t enforce it.
### Fix Focus Areas
- scripts/collate_changelog.py[60-68]
- scripts/collate_changelog.py[115-116]
- docs/changelog-fragments.md[6-15]
### What to change
- Filter/validate filenames in `collect()` (e.g., only `^\d+-.*\.md$`).
- If an unexpected `.md` is present: either skip it with a warning, or fail with exit code 1 without deleting anything.
- Consider adding a test covering the presence of an unexpected `.md` in `changelog.d/`.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
| # Insert directly BELOW [Unreleased] so Unreleased stays empty and on top, | ||
| # which is what the release train expects on the next cycle. | ||
| anchor = UNRELEASED + "\n" | ||
| text = text.replace(anchor, anchor + "\n" + section, 1) |
There was a problem hiding this comment.
WARNING: If [Unreleased] contains bullets, they end up under the new release section
text.replace(anchor, ...) inserts the new release section immediately after the ## [Unreleased] header. Any bullets already under [Unreleased] will then fall under the new ## [version] header in markdown, because it becomes the nearest preceding heading. The script assumes [Unreleased] is empty at release time but does not document or enforce this.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| if not FRAGMENT_DIR.is_dir(): | ||
| print("collate-changelog: no changelog.d/ directory, nothing to do") | ||
| return 0 | ||
| merged, consumed = collect(FRAGMENT_DIR) |
There was a problem hiding this comment.
SUGGESTION: collect() can raise ValueError for a malformed fragment, but main() does not catch it
If a fragment has no bullets, parse_fragment() raises ValueError. Since main() calls collect() without a try/except, the script crashes with an unhandled exception and traceback instead of exiting cleanly with code 1 as documented.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
Code Review SummaryStatus: 2 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
SUGGESTION
Files Reviewed (7 files)
Fix these issues in Kilo Cloud Reviewed by step-3.7-flash · Input: 113.4K · Output: 25.5K · Cached: 703.6K |
1 participant
Closes the changelog rebase treadmill. Implements tsk-ziomck.
THE PROBLEM, measured rather than assumed: every PR that edits CHANGELOG.md writes at the same [Unreleased] anchor, so any two concurrent PRs conflict there by construction and each merge re-conflicts the next. With N PRs open that is O(N^2) rebases which touch no real code. On 2026-08-04 I rebased #2275, #2281, #2282 and #2283 for changelog-only conflicts - seven rebases in one session, #2283 three times.
THE CHANGE:
PROVEN BOTH DIRECTIONS AGAINST THE REAL GATE, not just unit fixtures:
A MISTAKE WORTH RECORDING, because it is exactly the failure this repo keeps hitting: my first version put the convention doc at changelog.d/README.md. That file MATCHES changelog.d/*.md, so the readme satisfied the changelog rule - the probe above went GREEN on a change with no changelog at all. The gate would have shipped looking correct while being permanently satisfiable by touching a readme. The doc now lives at docs/changelog-fragments.md and changelog.d/ holds nothing but fragments and a .gitkeep. Both the config comment and the test name record why.
TESTS: 33 passing across tests/test_doc_gate.py (5 new, including the not-inert case and two glob-precision cases) and tests/test_collate_changelog.py (7, covering fold+delete, section grouping and ordering, wrapped-bullet continuation, dry-run, the no-fragments no-op, and a missing [Unreleased] anchor failing loudly WITHOUT eating the fragments).
Dogfooded: this PR's own changelog entry is a fragment.