Merged
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:
WalkthroughThis pull request introduces a "super chapters" feature enabling hierarchical organization of release notes by grouping regular chapters under higher-level category headings. The implementation adds new action inputs ( Changes
Sequence DiagramsequenceDiagram
participant Action as GitHub Action
participant Input as ActionInputs
participant CC as CustomChapters
participant HRecord as HierarchyIssueRecord
participant Output as Release Notes
Action->>Input: get_super_chapters()
Input->>Input: Parse YAML, validate entries
Input-->>Action: Return list[dict] with{title, labels}
Action->>CC: __init__(super_chapters=...)
CC->>CC: Initialize _super_chapters list
Action->>CC: populate(records)
CC->>CC: Track record labels via get_labels()
Action->>CC: to_string()
alt Has super chapters
CC->>CC: For each super chapter by label
CC->>HRecord: to_chapter_row(label_filter=super_labels)
HRecord->>HRecord: has_matching_labels(label_filter)
HRecord->>HRecord: Filter sub-hierarchy/sub-issues by labels
HRecord-->>CC: Filtered markdown row
CC->>Output: Render nested heading structure
CC->>CC: Collect unmatched records
CC->>Output: Render ## Uncategorized section
else No super chapters
CC->>Output: Render flat chapter structure
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 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 |
Contributor
There was a problem hiding this comment.
Pull request overview
Adds support for super chapters to the release notes generator, allowing users to group existing custom chapters under higher-level ## headings based on labels, while preserving the original flat ### rendering when the feature is not configured.
Changes:
- Introduces a new optional
super-chaptersaction input (YAML) and wires it throughActionInputs. - Extends
CustomChaptersrendering to support super-chapter grouping plus an## Uncategorizedfallback. - Adds unit tests and updates documentation/action metadata for the new input.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
action.yml |
Adds the super-chapters input and exports it as INPUT_SUPER_CHAPTERS for runtime consumption. |
release_notes_generator/action_inputs.py |
Implements ActionInputs.get_super_chapters() and logs parsed super-chapter config during validation. |
release_notes_generator/chapters/custom_chapters.py |
Adds super-chapter parsing and grouped rendering logic with an Uncategorized fallback. |
release_notes_generator/utils/constants.py |
Adds SUPER_CHAPTERS input key constant and extracts UNCATEGORIZED_CHAPTER_TITLE. |
docs/features/custom_chapters.md |
Documents configuration, rendering, behavior, and validation rules for super chapters. |
docs/configuration_reference.md |
Adds super-chapters to the inputs reference table. |
tests/unit/release_notes_generator/test_action_inputs.py |
Adds unit tests covering parsing/validation behavior for get_super_chapters(). |
tests/unit/release_notes_generator/chapters/test_custom_chapters.py |
Adds unit tests for grouped super-chapter output behavior and fallback behavior. |
tests/unit/conftest.py |
Adds a helper to construct CustomChapters instances with super-chapter behavior patched in tests. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
action.yml (1)
33-38: Consider documenting uncategorized fallback in the input description.You already document matching and multi-membership; adding one line about the automatic
Uncategorizedsection would make behavior complete directly inaction.yml.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@action.yml` around lines 33 - 38, Update the description for the super-chapters input in action.yml to mention the automatic fallback: add a sentence to the existing description block (the `description` key for the super-chapter array) stating that any records not matching any super-chapter label will be placed into an automatic "Uncategorized" section; keep it brief and match the tone of the surrounding bullets.release_notes_generator/chapters/custom_chapters.py (1)
235-244: Temporary mutation ofchapter.rowsis a code smell.While the
try/finallyensures restoration, directly mutating shared state can be fragile and isn't thread-safe. Consider either passing filter criteria toChapter.to_stringor creating a lightweight wrapper that yields filtered rows without mutation.Also, the dual check
str(rid) in matching_ids or rid in matching_idssuggests type inconsistency between row IDs (possibly int) andmatching_ids(always str). Consider normalizing ID types upstream for cleaner comparisons.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@release_notes_generator/chapters/custom_chapters.py` around lines 235 - 244, The helper _render_chapter_for_ids currently mutates chapter.rows; instead avoid shared-state mutation by building a filtered_rows dict and invoking Chapter.to_string on a non-mutated object—either (A) add an optional parameter to Chapter.to_string like rows/filter_ids and pass filtered_rows, or (B) create a lightweight shallow copy of the Chapter (e.g., new_chapter = Chapter(...same metadata..., rows=filtered_rows) or add a Chapter.with_rows(rows) factory) and call new_chapter.to_string; also normalize ID comparisons by converting matching_ids to a consistent type (e.g., set[str] = {str(x) for x in matching_ids}) or casting rid once when creating filtered_rows so you can remove the dual check (str(rid) in matching_ids or rid in matching_ids).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/features/custom_chapters.md`:
- Line 258: Update the sentence "Within each super chapter, records are routed
to regular chapters by the normal label matching rules." to use a hyphenated
compound adjective for clarity — change "label matching rules" to
"label-matching rules" so the line reads: "Within each super chapter, records
are routed to regular chapters by the normal label-matching rules."
In `@release_notes_generator/action_inputs.py`:
- Around line 175-195: get_super_chapters currently only YAML-parses the input
and leaves semantic validation to CustomChapters._parse_super_chapters; move
that full validation into the ActionInputs boundary by having get_super_chapters
perform the same semantic checks and return only validated structures
(list[dict[str,str]]). Specifically, extend get_super_chapters to validate each
entry's type and required keys/fields (the same contract enforced in
CustomChapters._parse_super_chapters), log and skip or error on invalid entries
consistently, and ensure the return value is strictly the validated list so
CustomChapters no longer needs to re-validate; update/remove the redundant
checks in CustomChapters._parse_super_chapters accordingly.
In `@release_notes_generator/chapters/custom_chapters.py`:
- Around line 367-399: Change the _parse_super_chapters signature to accept
mixed YAML types (use list[dict[str, Any]] for raw_super_chapters) and validate
the title before using it: after extracting entry["title"] in
_parse_super_chapters, ensure it's a str (if not, logger.warning and continue),
and keep using SuperChapter(title=title, labels=...) and _normalize_labels as
before; also adjust the local typing for labels_input to reflect str | list[str]
if you keep annotations.
---
Nitpick comments:
In `@action.yml`:
- Around line 33-38: Update the description for the super-chapters input in
action.yml to mention the automatic fallback: add a sentence to the existing
description block (the `description` key for the super-chapter array) stating
that any records not matching any super-chapter label will be placed into an
automatic "Uncategorized" section; keep it brief and match the tone of the
surrounding bullets.
In `@release_notes_generator/chapters/custom_chapters.py`:
- Around line 235-244: The helper _render_chapter_for_ids currently mutates
chapter.rows; instead avoid shared-state mutation by building a filtered_rows
dict and invoking Chapter.to_string on a non-mutated object—either (A) add an
optional parameter to Chapter.to_string like rows/filter_ids and pass
filtered_rows, or (B) create a lightweight shallow copy of the Chapter (e.g.,
new_chapter = Chapter(...same metadata..., rows=filtered_rows) or add a
Chapter.with_rows(rows) factory) and call new_chapter.to_string; also normalize
ID comparisons by converting matching_ids to a consistent type (e.g., set[str] =
{str(x) for x in matching_ids}) or casting rid once when creating filtered_rows
so you can remove the dual check (str(rid) in matching_ids or rid in
matching_ids).
🪄 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: b625f1e8-a06c-426d-9639-b30f9cd8a5dc
📒 Files selected for processing (9)
action.ymldocs/configuration_reference.mddocs/features/custom_chapters.mdrelease_notes_generator/action_inputs.pyrelease_notes_generator/chapters/custom_chapters.pyrelease_notes_generator/utils/constants.pytests/unit/conftest.pytests/unit/release_notes_generator/chapters/test_custom_chapters.pytests/unit/release_notes_generator/test_action_inputs.py
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 `@release_notes_generator/action_inputs.py`:
- Around line 208-223: The code currently accepts any non-missing "title" value
which may be non-string or blank; update the validation around entry["title"] in
the super-chapter parsing block so that you only accept a title that is a
non-empty string (e.g., check isinstance(title, str) and title.strip() != ""),
log a warning and continue if the title is invalid, and only pass the
cleaned/stripped title into result.append({"title": title, "labels":
normalized}) (keep using normalize_labels and the existing logging patterns).
🪄 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: f83123e6-7884-4ada-81f2-3e9f9139277a
📒 Files selected for processing (7)
docs/features/custom_chapters.mdrelease_notes_generator/action_inputs.pyrelease_notes_generator/chapters/custom_chapters.pyrelease_notes_generator/utils/utils.pytests/unit/conftest.pytests/unit/release_notes_generator/chapters/test_custom_chapters.pytests/unit/release_notes_generator/test_action_inputs.py
✅ Files skipped from review due to trivial changes (1)
- docs/features/custom_chapters.md
🚧 Files skipped from review as they are similar to previous changes (4)
- tests/unit/release_notes_generator/test_action_inputs.py
- tests/unit/conftest.py
- tests/unit/release_notes_generator/chapters/test_custom_chapters.py
- release_notes_generator/chapters/custom_chapters.py
There was a problem hiding this comment.
♻️ Duplicate comments (1)
release_notes_generator/action_inputs.py (1)
211-226:⚠️ Potential issue | 🟡 MinorStrip the validated title before appending.
Line 212 validates that
title.strip()is non-empty, but Line 226 appends the originaltitlevalue with potential leading/trailing whitespace. This could produce headings like" Module A "in the release notes.Proposed fix
title = entry["title"] if not isinstance(title, str) or not title.strip(): logger.warning("Skipping super-chapter with invalid title value: %r", title) continue + title = title.strip() raw_labels = entry.get("labels", entry.get("label"))🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@release_notes_generator/action_inputs.py` around lines 211 - 226, The validated title may contain leading/trailing whitespace—after confirming title.strip() is non-empty, replace or assign a stripped version (e.g., title = title.strip() or use title_stripped) before appending; update the code that builds the output (the result.append({"title": title, "labels": normalized}) call) to use the stripped title so headings are normalized, leaving label normalization via normalize_labels unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@release_notes_generator/action_inputs.py`:
- Around line 211-226: The validated title may contain leading/trailing
whitespace—after confirming title.strip() is non-empty, replace or assign a
stripped version (e.g., title = title.strip() or use title_stripped) before
appending; update the code that builds the output (the result.append({"title":
title, "labels": normalized}) call) to use the stripped title so headings are
normalized, leaving label normalization via normalize_labels unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 4be0854c-caad-4b3c-8aab-3e5bef3e0bb2
📒 Files selected for processing (2)
release_notes_generator/action_inputs.pytests/unit/release_notes_generator/test_action_inputs.py
✅ Files skipped from review due to trivial changes (1)
- tests/unit/release_notes_generator/test_action_inputs.py
…ents (#280) * feat: enhance hierarchy issue rendering logic for open and closed parents * feat: add support for highlighting open sub-issues under closed hierarchy parents
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
tests/unit/release_notes_generator/utils/test_gh_action.py (1)
22-36: Cover the explicitdefault=path too.These assertions lock in the implicit
default=""call, but the contract change also depends on forwarding a caller-supplied default unchanged. A smallget_action_input("explicit-default", "fallback")case would keep that branch covered.🧪 Suggested test
+def test_get_input_with_explicit_default(mocker): + mock_getenv = mocker.patch("os.getenv", side_effect=lambda name, default=None: default) + + result = get_action_input("explicit-default", "fallback") + + mock_getenv.assert_called_with("INPUT_EXPLICIT_DEFAULT", default="fallback") + assert result == "fallback"As per coding guidelines, "Test return values, exceptions, log messages, and exit codes in Python tests".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/unit/release_notes_generator/utils/test_gh_action.py` around lines 22 - 36, Add a test that exercises the explicit default path for get_action_input: call get_action_input("explicit-default", "fallback") in tests/unit/release_notes_generator/utils/test_gh_action.py, mock os.getenv and assert it was called with the environment key "INPUT_EXPLICIT_DEFAULT" and default="fallback", and assert the function returns the mocked value; reference get_action_input to locate the implementation and the existing tests for pattern.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/features/custom_chapters.md`:
- Around line 218-315: The docs duplicate the entire "## Super Chapters" section
back-to-back; remove the second copy so there's a single consolidated "## Super
Chapters" section (look for the repeated "## Super Chapters" heading). In that
remaining section add a short sentence to Rendering or Behavior stating the
fallback: "Records that do not match any super-chapter labels are grouped under
## Uncategorized." Also ensure the Validation paragraph still appears once and
mentions skipped entries as before.
In `@release_notes_generator/chapters/custom_chapters.py`:
- Around line 421-425: The constructor is re-parsing already-validated
super-chapter entries causing duplicated validation; replace the duplicate calls
so self._super_chapters is assigned the validated, normalized output from
ActionInputs.get_super_chapters() only (do not call _parse_super_chapters on
that value), remove the redundant parsing calls around the constructor and the
block referenced (including the repeated section covering the 429-474 region),
and leave the standalone _parse_super_chapters method only for the original
raw-YAML parsing path if still needed; target symbols: self._super_chapters,
_parse_super_chapters, and ActionInputs.get_super_chapters.
- Around line 91-99: The class contains unresolved duplicate members—remove the
later duplicates and keep only the first occurrence of each: delete the second
__init__ duplicate and the later definitions of _to_string_with_super_chapters
and _parse_super_chapters; ensure the kept _to_string_with_super_chapters is the
first version that preserves the "## Uncategorized" fallback logic so unmatched
records are collected, and ensure the kept _parse_super_chapters is the first
version that does not re-validate/normalize input (leaving validation to
ActionInputs.get_super_chapters()); update imports/types only if needed after
removing the duplicates.
In `@release_notes_generator/model/record/hierarchy_issue_record.py`:
- Around line 225-254: The code that prefixes open-child icons currently
prepends the icon before the child's existing "- " list marker (see
sub_hierarchy_issue.to_chapter_row() handling and the open_icon_prefix /
sub_issue_block construction), which breaks nested Markdown and leaves stray
spaces when the icon is empty; change the logic to detect and preserve an
existing leading list marker (the initial "-" and following space) and insert
the icon immediately after that marker (e.g., "- " -> "- {icon} " only when icon
non-empty, otherwise leave as "- "), update both the sub_hierarchy_issue branch
and the sub_issue block where open_icon_prefix is used
(ActionInputs.get_open_hierarchy_sub_issue_icon()), and add regression
assertions that verify the rendered output begins with "- 🟡" when icon set and
remains "- " when icon is empty.
---
Nitpick comments:
In `@tests/unit/release_notes_generator/utils/test_gh_action.py`:
- Around line 22-36: Add a test that exercises the explicit default path for
get_action_input: call get_action_input("explicit-default", "fallback") in
tests/unit/release_notes_generator/utils/test_gh_action.py, mock os.getenv and
assert it was called with the environment key "INPUT_EXPLICIT_DEFAULT" and
default="fallback", and assert the function returns the mocked value; reference
get_action_input to locate the implementation and the existing tests for
pattern.
🪄 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: b4d7d817-034a-47e2-933f-b6b4ec8142c4
📒 Files selected for processing (11)
action.ymldocs/features/custom_chapters.mddocs/features/issue_hierarchy_support.mdrelease_notes_generator/action_inputs.pyrelease_notes_generator/chapters/custom_chapters.pyrelease_notes_generator/model/record/hierarchy_issue_record.pyrelease_notes_generator/utils/constants.pyrelease_notes_generator/utils/gh_action.pytests/unit/conftest.pytests/unit/release_notes_generator/model/test_hierarchy_issue_record.pytests/unit/release_notes_generator/utils/test_gh_action.py
🚧 Files skipped from review as they are similar to previous changes (4)
- action.yml
- release_notes_generator/action_inputs.py
- release_notes_generator/utils/constants.py
- tests/unit/conftest.py
…eRecord classes for improved readability
miroslavpojer
added
the
work in progress
…d classes for clarity
miroslavpojer
commented
Apr 7, 2026
Collaborator
Author
miroslavpojer
left a comment
miroslavpojer
left a comment
There was a problem hiding this comment.
I have done self review as I was writing it for a long time with pauses.
…es across multiple classes
tmikula-dev
reviewed
Apr 8, 2026
| - Prefer TDD workflow: | ||
| - Must create or update `SPEC.md` in the relevant package directory before writing any code, listing scenarios, inputs, and expected outputs. | ||
| - Must propose the full set of test cases (name + one-line intent + input summary + expected output summary) and wait for user confirmation before writing any code. | ||
| - Must be ready to add, remove, or rename tests based on user feedback before proceeding. |
Collaborator
There was a problem hiding this comment.
This line means, you are always doing a planning of task in PLAN mode before implementing? If not, would this line mean, that the implementation can stop to get confirmation from user and will spent extra premium requests.
From this section I am not sure, if TDD is now way how to implement new things, since it has a big part of the copilot doc, but is Preferred workflow.
Collaborator
Author
There was a problem hiding this comment.
The TDD block is marked Prefer (not Must), so it's being trialed rather than enforced.
- The plan-then-confirm checkpoint is useful for bigger changes but too heavy for quick fixes
- Worth tracking whether the extra round-trip pays off in practice
Final Note: I am experimenting with this approach, here.
tmikula-dev
reviewed
Apr 8, 2026
| def _collect_super_chapter_ids(self, sc: SuperChapter) -> set[str]: | ||
| """Return record IDs whose labels match the given super chapter.""" | ||
| matching: set[str] = set() | ||
| for rid, labels in self._record_labels.items(): |
Collaborator
There was a problem hiding this comment.
What rid stands for please?
Collaborator
Author
There was a problem hiding this comment.
Short for record ID — the unique PR or issue number used as the key in _record_labels.
tmikula-dev
reviewed
Apr 8, 2026
| result.append({"title": title, "labels": normalized}) | ||
| logger.debug("Validated super-chapter '%s' with labels: %s", title, normalized) | ||
| ActionInputs._super_chapters_raw = super_chapters_input | ||
| ActionInputs._super_chapters_cache = result |
Collaborator
There was a problem hiding this comment.
What is this cache var solving?
Collaborator
Author
There was a problem hiding this comment.
get_super_chapters() reads and parses a YAML env var. The cache avoids re-parsing the same string on every call.
tmikula-dev
approved these changes
Apr 8, 2026
Collaborator
tmikula-dev
left a comment
tmikula-dev
left a comment
There was a problem hiding this comment.
The logic looks correct. It is complex huge feature/refactor. During review I was little lost with the naming like coh, rid etc. But since you are the key developer, it is important that you are familiar with them. I left few comments, to understand some stuff, but approving the logic.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Overview
Adds
super-chaptersas a new optional input that lets users group existing custom chapters under top-level##headings in the release notes output.Previously, all custom chapters rendered as flat
###headings. With super chapters, users can declare label-based groupings (e.g. "Module A", "Module B") that wrap the matching records under a##heading, with the regular chapter titles nested as###inside. Records not matched by any super chapter fall through to an## Uncategorizedsection.Key changes:
ActionInputs.get_super_chapters()parses the newsuper-chaptersYAML input (mirrorsget_chapters()pattern)CustomChapters._to_string_with_super_chapters()refactored into focused helpers and extended with an uncategorized fallback_record_labelsnow tracks all records (including label-less COH-routed ones) so they appear in the fallbackUNCATEGORIZED_CHAPTER_TITLEconstant extracted to keep the contract-visible string stableRelease Notes
super-chaptersinput: group release note chapters under top-level##headings by label## Uncategorizedfallback sectionRelated
Closes #99
Summary by CodeRabbit
New Features
Documentation