feat: add service chapter exclusion rules for release notes generation#304
feat: add service chapter exclusion rules for release notes generation#304miroslavpojer merged 7 commits intomasterfrom
Conversation
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 23 minutes and 1 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
WalkthroughAdds a new Changes
Sequence DiagramsequenceDiagram
participant Builder as ReleaseNotesBuilder
participant Inputs as ActionInputs
participant Chapters as ServiceChapters
participant Pop as populate()
participant Helper as _is_excluded_from_chapter()
Builder->>Inputs: get_service_chapter_exclude()
Inputs-->>Builder: dict[chapter_title, list[list[str]]]
Builder->>Chapters: __init__(..., chapter_exclude=...)
Chapters->>Chapters: store global & per-chapter groups
Builder->>Pop: populate(records, chapters)
loop for each record
Pop->>Chapters: _is_globally_excluded(record)
alt global match
Chapters-->>Pop: True (skip record)
else no global match
loop for each chapter
Pop->>Helper: _is_excluded_from_chapter(record, chapter)
Helper->>Chapters: _matches_any_group(labels, groups)
alt match
Helper-->>Pop: True (skip this chapter)
else no match
Pop->>Chapters: add_row(record, chapter)
end
end
end
end
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Possibly related PRs
Suggested Reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
tests/unit/release_notes_generator/chapters/test_service_chapters.py (1)
294-317: UseGLOBAL_EXCLUDE_KEYconstant instead of literal"*"in testsUsing the constant here reduces drift risk if the reserved key ever changes and keeps tests aligned with production constants.
Proposed refactor
from release_notes_generator.utils.constants import ( + GLOBAL_EXCLUDE_KEY, MERGED_PRS_LINKED_TO_NOT_CLOSED_ISSUES, CLOSED_ISSUES_WITHOUT_PULL_REQUESTS, CLOSED_ISSUES_WITHOUT_USER_DEFINED_LABELS, @@ def test_populate_global_full_match_excluded_from_all(record_with_issue_closed_no_pull): @@ - exclude_rules={"*": [["label1", "label2"]]}, + exclude_rules={GLOBAL_EXCLUDE_KEY: [["label1", "label2"]]}, @@ def test_populate_global_partial_and_failure(record_with_issue_closed_no_pull): @@ - exclude_rules={"*": [["label1", "nonexistent"]]}, + exclude_rules={GLOBAL_EXCLUDE_KEY: [["label1", "nonexistent"]]}, @@ def test_populate_global_precedes_per_chapter(record_with_issue_closed_no_pull): @@ - "*": [["label1", "label2"]], + GLOBAL_EXCLUDE_KEY: [["label1", "label2"]],🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/unit/release_notes_generator/chapters/test_service_chapters.py` around lines 294 - 317, Replace the literal "*" used as the global exclusion key in the tests with the GLOBAL_EXCLUDE_KEY constant from the same module that defines ServiceChapters; update the test cases (e.g., in test_populate_global_partial_and_failure and test_populate_global_precedes_per_chapter) to use GLOBAL_EXCLUDE_KEY as the key in exclude_rules and add the corresponding import for GLOBAL_EXCLUDE_KEY alongside ServiceChapters and CLOSED_ISSUES_WITHOUT_PULL_REQUESTS so the tests rely on the canonical reserved key rather than a hardcoded string.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@action.yml`:
- Around line 104-117: The composite action fails to forward the
service-chapter-exclude input to the runtime env, so
get_action_input("service-chapter-exclude") won't see
INPUT_SERVICE_CHAPTER_EXCLUDE; add the INPUT_SERVICE_CHAPTER_EXCLUDE environment
variable to the composite step env block and set it to the corresponding input
(i.e., env entry INPUT_SERVICE_CHAPTER_EXCLUDE: ${{
inputs.service-chapter-exclude }}) so the Python code can read the value via
get_action_input("service-chapter-exclude").
In `@docs/features/service_chapters.md`:
- Around line 120-123: Update the wording under the "Label-Based Exclusion
Rules" section so it does not refer only to "issues": change instances that say
the rules "filter out issues" or "excludes a matching record from that chapter
only" to more inclusive phrasing such as "records (issues or PRs)" or simply
"records" to reflect that `service-chapter-exclude` applies to both issue- and
PR-based service chapters; keep the explanation of AND/OR logic and the
`service-chapter-exclude` term intact and ensure the short example line (the
“Per-chapter exclusion” sentence) uses the new inclusive term.
---
Nitpick comments:
In `@tests/unit/release_notes_generator/chapters/test_service_chapters.py`:
- Around line 294-317: Replace the literal "*" used as the global exclusion key
in the tests with the GLOBAL_EXCLUDE_KEY constant from the same module that
defines ServiceChapters; update the test cases (e.g., in
test_populate_global_partial_and_failure and
test_populate_global_precedes_per_chapter) to use GLOBAL_EXCLUDE_KEY as the key
in exclude_rules and add the corresponding import for GLOBAL_EXCLUDE_KEY
alongside ServiceChapters and CLOSED_ISSUES_WITHOUT_PULL_REQUESTS so the tests
rely on the canonical reserved key rather than a hardcoded string.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e93c082d-a61d-4e5e-b367-d8c04385a02d
📒 Files selected for processing (9)
action.ymldocs/configuration_reference.mddocs/features/service_chapters.mdrelease_notes_generator/action_inputs.pyrelease_notes_generator/builder/builder.pyrelease_notes_generator/chapters/service_chapters.pyrelease_notes_generator/utils/constants.pytests/unit/release_notes_generator/chapters/test_service_chapters.pytests/unit/release_notes_generator/test_action_inputs.py
There was a problem hiding this comment.
🧹 Nitpick comments (1)
.github/workflows/test.yml (1)
212-212: Harden assertions now that empty chapters are enabled.With
INPUT_PRINT_EMPTY_CHAPTERS='true'(Line 212), the header check can pass even when those chapters have no entries. Consider validating against the generated release-notes section only, and assert at least one bullet entry there.Proposed test hardening
- name: Run action against real repository and validate output env: @@ INPUT_WARNINGS: 'true' INPUT_PRINT_EMPTY_CHAPTERS: 'true' @@ run: | @@ # Display output for debugging echo "=== Action Output ===" cat output.txt echo "=== End of Output ===" + + # Isolate generated release notes body (exclude verbose logs) + awk '/Generated release notes:/{flag=1; next} flag {print}' output.txt > release_notes.txt problems=() @@ # Check for markdown chapter headers - if ! grep -qE '^### (Breaking Changes 💥|New Features 🎉|Bugfixes 🛠)' output.txt; then + if ! grep -qE '^### (Breaking Changes 💥|New Features 🎉|Bugfixes 🛠)' release_notes.txt; then problems+=("missing expected chapter headers") fi - # Check for issue/PR references (e.g., `#123`) - if ! grep -qE '#[0-9]+' output.txt; then - problems+=("missing issue/PR references") + # Ensure at least one release note bullet references an issue/PR + if ! grep -qE '^- .*#[0-9]+' release_notes.txt; then + problems+=("missing issue/PR references in release notes bullets") fi - # Check for developer mentions (e.g., `@username`) - if ! grep -qE '@[a-zA-Z0-9_-]+' output.txt; then - problems+=("missing developer mentions") + # Ensure at least one release note bullet contains a developer mention + if ! grep -qE '^- .*@[a-zA-Z0-9_-]+' release_notes.txt; then + problems+=("missing developer mentions in release notes bullets") fi🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/test.yml at line 212, The current CI test allows empty chapters because INPUT_PRINT_EMPTY_CHAPTERS is true; update the test that performs the header check to instead parse the generated "release-notes" section (look for the "## Release Notes" heading or the release-notes output block) and assert that this section contains at least one bullet entry (e.g., a line starting with "-" or "*"); keep the existing header validation but change the assertion target from overall headers to the parsed release-notes content and add an explicit non-empty bullets assertion to ensure at least one entry is present when INPUT_PRINT_EMPTY_CHAPTERS is enabled.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In @.github/workflows/test.yml:
- Line 212: The current CI test allows empty chapters because
INPUT_PRINT_EMPTY_CHAPTERS is true; update the test that performs the header
check to instead parse the generated "release-notes" section (look for the "##
Release Notes" heading or the release-notes output block) and assert that this
section contains at least one bullet entry (e.g., a line starting with "-" or
"*"); keep the existing header validation but change the assertion target from
overall headers to the parsed release-notes content and add an explicit
non-empty bullets assertion to ensure at least one entry is present when
INPUT_PRINT_EMPTY_CHAPTERS is enabled.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d2b0d1bb-6733-4e5c-b0a5-9de7ad023790
📒 Files selected for processing (3)
.github/workflows/test.ymlaction.ymldocs/features/service_chapters.md
🚧 Files skipped from review as they are similar to previous changes (2)
- action.yml
- docs/features/service_chapters.md
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
release_notes_generator/chapters/service_chapters.py (1)
193-210:⚠️ Potential issue | 🟠 MajorPer-chapter exclusion currently suppresses fallback routing unintentionally.
On Line 198, Line 210, Line 252, and Line 263,
populated/consumedis set even when a record is excluded from that chapter. That makes the record skipOTHERS_NO_TOPIC, so a chapter-level exclusion can behave like a full drop.💡 Suggested fix
@@ if pulls_count == 0: if not self._is_excluded_from_chapter(record, CLOSED_ISSUES_WITHOUT_PULL_REQUESTS): record.add_to_chapter_presence(CLOSED_ISSUES_WITHOUT_PULL_REQUESTS) self.chapters[CLOSED_ISSUES_WITHOUT_PULL_REQUESTS].add_row(record_id, record.to_chapter_row()) self.used_record_numbers.append(record_id) - populated = True + populated = True @@ if not self._is_excluded_from_chapter(record, CLOSED_ISSUES_WITHOUT_USER_DEFINED_LABELS): record.add_to_chapter_presence(CLOSED_ISSUES_WITHOUT_USER_DEFINED_LABELS) self.chapters[CLOSED_ISSUES_WITHOUT_USER_DEFINED_LABELS].add_row(record_id, record.to_chapter_row()) self.used_record_numbers.append(record_id) - populated = True + populated = True @@ if not self._is_excluded_from_chapter(record, MERGED_PRS_WITHOUT_ISSUE_AND_USER_DEFINED_LABELS): record.add_to_chapter_presence(MERGED_PRS_WITHOUT_ISSUE_AND_USER_DEFINED_LABELS) self.chapters[MERGED_PRS_WITHOUT_ISSUE_AND_USER_DEFINED_LABELS].add_row( record_id, record.to_chapter_row() ) self.used_record_numbers.append(record_id) - consumed = True + consumed = True @@ if not self._is_excluded_from_chapter(record, MERGED_PRS_LINKED_TO_NOT_CLOSED_ISSUES): record.add_to_chapter_presence(MERGED_PRS_LINKED_TO_NOT_CLOSED_ISSUES) self.chapters[MERGED_PRS_LINKED_TO_NOT_CLOSED_ISSUES].add_row(record_id, record.to_chapter_row()) self.used_record_numbers.append(record_id) - consumed = True + consumed = TrueAlso applies to: 246-253, 259-266
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@release_notes_generator/chapters/service_chapters.py` around lines 193 - 210, The per-chapter exclusion check (_is_excluded_from_chapter) currently only guards adding the record to the chapter, but populated/consumed and used_record_numbers are set unconditionally afterwards, causing excluded records to be treated as consumed and skip fallback (e.g., OTHERS_NO_TOPIC). Fix each affected block (e.g., the CLOSED_ISSUES_WITHOUT_PULL_REQUESTS and CLOSED_ISSUES_WITHOUT_USER_DEFINED_LABELS sections) so that add_to_chapter_presence, chapters[...].add_row, used_record_numbers.append(record_id) and setting populated = True only occur inside the branch where the record is NOT excluded; leave duplicity/early-return checks (calls to __is_row_present and duplicity_allowed) unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@release_notes_generator/chapters/service_chapters.py`:
- Around line 193-210: The per-chapter exclusion check
(_is_excluded_from_chapter) currently only guards adding the record to the
chapter, but populated/consumed and used_record_numbers are set unconditionally
afterwards, causing excluded records to be treated as consumed and skip fallback
(e.g., OTHERS_NO_TOPIC). Fix each affected block (e.g., the
CLOSED_ISSUES_WITHOUT_PULL_REQUESTS and
CLOSED_ISSUES_WITHOUT_USER_DEFINED_LABELS sections) so that
add_to_chapter_presence, chapters[...].add_row,
used_record_numbers.append(record_id) and setting populated = True only occur
inside the branch where the record is NOT excluded; leave duplicity/early-return
checks (calls to __is_row_present and duplicity_allowed) unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f5177834-0cc6-4dc0-a6d8-32829ae9ec77
📒 Files selected for processing (4)
action.ymlrelease_notes_generator/builder/builder.pyrelease_notes_generator/chapters/service_chapters.pytests/unit/release_notes_generator/chapters/test_service_chapters.py
✅ Files skipped from review due to trivial changes (1)
- release_notes_generator/builder/builder.py
🚧 Files skipped from review as they are similar to previous changes (2)
- action.yml
- tests/unit/release_notes_generator/chapters/test_service_chapters.py
tmikula-dev
left a comment
There was a problem hiding this comment.
Please see the comments.
tmikula-dev
left a comment
There was a problem hiding this comment.
Please see the comments.
Overview
Adds label-based exclusion rules for service chapters, giving users fine-grained control over which issues/PRs appear in each service chapter based on their labels.
Previously the only way to remove entries from service chapters was to hide the entire chapter. With this change users can define per-chapter or global label rules using AND/OR logic:
"*"key): a matching record is dropped from all service chapters entirely.Changed files:
action.yml— newservice-chapter-excludeinputrelease_notes_generator/action_inputs.py— newActionInputs.get_service_chapter_exclude()method with YAML parsing and validationrelease_notes_generator/chapters/service_chapters.py— newexclude_rulesconstructor parameter; new helpers_is_globally_excluded,_is_excluded_from_chapter,_matches_any_grouprelease_notes_generator/builder/builder.py— wires the new input intoServiceChaptersrelease_notes_generator/utils/constants.py— new constantsGLOBAL_EXCLUDE_KEY,SERVICE_CHAPTER_EXCLUDEdocs/features/service_chapters.md— new "Label-Based Exclusion Rules" sectiondocs/configuration_reference.md— reference entry for the new inputtests/unit/release_notes_generator/chapters/test_service_chapters.py— tests for per-chapter and global exclusion rules; two duplicate tests removedtests/unit/release_notes_generator/test_action_inputs.py— tests forget_service_chapter_exclude()parsing and validationRelease Notes
service-chapter-excludeaction input: label-based exclusion rules (AND/OR logic) for filtering records out of individual or all service chapters."*"global key support inservice-chapter-excludeto drop matching records from every service chapter at once.Related
Closes #298
Summary by CodeRabbit
New Features
service-chapter-excludeinput to exclude records from service chapters using label-groups (AND within groups, OR across groups); supports a reserved "*" key for global exclusions and is honored when warnings are enabled.Documentation
Tests
Chores