From de66dc711c2d9b4bb0b04c9291538d2ece6f5bf3 Mon Sep 17 00:00:00 2001 From: miroslavpojer Date: Thu, 2 Apr 2026 10:17:56 +0200 Subject: [PATCH 01/20] feat: add super-chapters support for grouping chapters by label --- action.yml | 9 ++ docs/configuration_reference.md | 1 + docs/features/custom_chapters.md | 49 +++++++ release_notes_generator/action_inputs.py | 26 ++++ .../chapters/custom_chapters.py | 104 +++++++++++++++ release_notes_generator/utils/constants.py | 1 + tests/unit/conftest.py | 28 ++++ .../chapters/test_custom_chapters.py | 122 +++++++++++++++--- 8 files changed, 320 insertions(+), 20 deletions(-) diff --git a/action.yml b/action.yml index dbf935ee..674b3e20 100644 --- a/action.yml +++ b/action.yml @@ -29,6 +29,14 @@ inputs: At most one `catch-open-hierarchy` chapter allowed; duplicates are warned and ignored. required: false default: '' + super-chapters: + description: | + YAML array of super-chapter definitions that group regular chapters by label. + Required keys: `title`, `label` or `labels`. + Records matching a super-chapter label are rendered inside that super-chapter. + A record can appear in multiple super-chapters. + required: false + default: '' duplicity-scope: description: 'Allow duplicity of issue lines in chapters. Scopes: custom, service, both, none.' required: false @@ -177,6 +185,7 @@ runs: INPUT_GITHUB_TOKEN: ${{ env.GITHUB_TOKEN }} INPUT_TAG_NAME: ${{ inputs.tag-name }} INPUT_CHAPTERS: ${{ inputs.chapters }} + INPUT_SUPER_CHAPTERS: ${{ inputs.super-chapters }} INPUT_FROM_TAG_NAME: ${{ inputs.from-tag-name }} INPUT_HIERARCHY: ${{ inputs.hierarchy }} INPUT_DUPLICITY_SCOPE: ${{ inputs.duplicity-scope }} diff --git a/docs/configuration_reference.md b/docs/configuration_reference.md index 2a19275b..02b778f8 100644 --- a/docs/configuration_reference.md +++ b/docs/configuration_reference.md @@ -27,6 +27,7 @@ This page lists all action inputs and outputs with defaults. Grouped for readabi | `row-format-issue` | No | `{type}: {number} _{title}_ developed by {developers} in {pull-requests}` | Template for issue rows. | | `row-format-pr` | No | `{number} _{title}_ developed by {developers}` | Template for PR rows. | | `row-format-link-pr` | No | `true` | If true adds `PR:` prefix when a PR is listed without an issue. | +| `super-chapters` | No | "" | YAML multi-line list of super-chapter entries (`title` + `label`/`labels`). Groups regular chapters under higher-level headings by label. See [Super Chapters](features/custom_chapters.md#super-chapters). | > CodeRabbit summaries must already be present in the PR body (produced by your own CI/App setup). This action only parses existing summaries; it does not configure or call CodeRabbit. diff --git a/docs/features/custom_chapters.md b/docs/features/custom_chapters.md index f666aeec..f6fdf9c0 100644 --- a/docs/features/custom_chapters.md +++ b/docs/features/custom_chapters.md @@ -215,6 +215,55 @@ A `catch-open-hierarchy` chapter can also be `hidden: true` to silently track op - Duplicate `catch-open-hierarchy: true` chapters are reduced to the first; a warning is logged for the rest. - When `hierarchy: false`, a warning is logged once at populate time: `"catch-open-hierarchy has no effect when hierarchy is disabled"`. +## Super Chapters + +**Super chapters** group regular chapters under higher-level headings based on a separate label. This is useful in monorepo or multi-module projects where the same chapter structure (Features, Bugfixes, …) should appear once per component. + +### Configuration + +Define super chapters via the `super-chapters` input — a YAML array with `title` and `label`/`labels`: + +```yaml +with: + super-chapters: | + - title: "Atum Server" + label: "atum-server" + - title: "Atum Agent" + labels: "atum-agent, atum-agent-spark" + chapters: | + - {"title": "Enhancements", "label": "enhancement"} + - {"title": "Bugfixes", "label": "bug"} +``` + +### Rendering + +When super chapters are configured the output uses `##` headings for super chapters and `###` for regular chapters nested inside: + +```markdown +## Atum Server +### Enhancements +- #10 _Streaming API_ developed by @alice in #12 + +### Bugfixes +- #15 _Fix timeout_ developed by @bob in #16 + +## Atum Agent +### Enhancements +- #20 _Checkpointing_ developed by @carol in #22 +``` + +### Behavior +- A record is placed under a super chapter if it carries at least one label matching the super chapter's labels. +- A record can appear in **multiple** super chapters if its labels match more than one. +- Within each super chapter, records are routed to regular chapters by the normal label matching rules. +- Empty super chapters (no matching records) respect the `print-empty-chapters` setting. +- When no super chapters are configured, output is flat (unchanged from previous behavior). + +### Validation +- Entries missing `title` or `label`/`labels` are skipped with a warning. +- Non-dict entries are skipped with a warning. +- Empty labels after normalization cause the entry to be skipped with a warning. + ## Related Features - [Duplicity Handling](./duplicity_handling.md) – governs multi-chapter visibility. - [Release Notes Extraction](./release_notes_extraction.md) – provides the change increment lines. diff --git a/release_notes_generator/action_inputs.py b/release_notes_generator/action_inputs.py index ec6555c7..c5716eaa 100644 --- a/release_notes_generator/action_inputs.py +++ b/release_notes_generator/action_inputs.py @@ -30,6 +30,7 @@ GITHUB_TOKEN, TAG_NAME, CHAPTERS, + SUPER_CHAPTERS, PUBLISHED_AT, VERBOSE, WARNINGS, @@ -171,6 +172,30 @@ def get_chapters() -> list[dict[str, str]]: return chapters + @staticmethod + def get_super_chapters() -> list[dict[str, str]]: + """ + Get list of super chapter definitions from the action inputs. + + Returns: + Parsed YAML list of dicts with 'title' and 'label'/'labels' keys, + or an empty list when the input is absent or invalid. + """ + raw: str = get_action_input(SUPER_CHAPTERS, default="") # type: ignore[assignment] + if not raw or not raw.strip(): + return [] + + try: + parsed = yaml.safe_load(raw) + if not isinstance(parsed, list): + logger.error("Error: 'super-chapters' input is not a valid YAML list.") + return [] + except yaml.YAMLError as exc: + logger.error("Error parsing 'super-chapters' input: {%s}", exc) + return [] + + return parsed + @staticmethod def get_hierarchy() -> bool: """ @@ -546,6 +571,7 @@ def validate_inputs() -> None: logger.debug("CodeRabbit summary ignore groups: %s", coderabbit_summary_ignore_groups) logger.debug("Hidden service chapters: %s", ActionInputs.get_hidden_service_chapters()) logger.debug("Service chapter order: %s", ActionInputs.get_service_chapter_order()) + logger.debug("Super chapters: %s", ActionInputs.get_super_chapters()) @staticmethod def _detect_row_format_invalid_keywords(row_format: str, row_type: str = "Issue", clean: bool = False) -> str: diff --git a/release_notes_generator/chapters/custom_chapters.py b/release_notes_generator/chapters/custom_chapters.py index 0975fad7..b1f1a485 100644 --- a/release_notes_generator/chapters/custom_chapters.py +++ b/release_notes_generator/chapters/custom_chapters.py @@ -20,6 +20,7 @@ """ import logging +from dataclasses import dataclass, field from typing import Any, Iterable from release_notes_generator.action_inputs import ActionInputs @@ -32,6 +33,14 @@ logger = logging.getLogger(__name__) +@dataclass +class SuperChapter: + """A label-based grouping that wraps regular chapters in the rendered output.""" + + title: str + labels: list[str] = field(default_factory=list) + + def _normalize_labels(raw: Any) -> list[str]: # helper for multi-label """Normalize a raw labels definition into an ordered, de-duplicated list. @@ -75,6 +84,11 @@ class CustomChapters(BaseChapters): A class used to represent the custom chapters in the release notes. """ + def __init__(self, sort_ascending: bool = True, print_empty_chapters: bool = True): + super().__init__(sort_ascending, print_empty_chapters) + self._super_chapters: list[SuperChapter] = [] + self._record_labels: dict[str, list[str]] = {} + def _find_catch_open_hierarchy_chapter(self) -> Chapter | None: """Return the first chapter with catch_open_hierarchy enabled, or None.""" for ch in self.chapters.values(): @@ -136,6 +150,10 @@ def populate(self, records: dict[str, Record]) -> None: record_labels = getattr(record, "labels", []) + # Track labels for super-chapter grouping at render time + if record_labels: + self._record_labels[record_id] = list(record_labels) + # Conditional Custom Chapter gate: intercept open hierarchy parents before label routing. # Note: precedes the record_labels early-exit so label-less HierarchyIssueRecord # is still routed to a no-filter COH chapter. @@ -175,9 +193,21 @@ def to_string(self) -> str: """ Converts the custom chapters to a string, excluding hidden chapters. + When super chapters are configured, records are grouped under super-chapter + headings (``## Title``) with regular chapters nested inside (``### Title``). + A record may appear in multiple super chapters. Chapters with no matching + records under a given super chapter are governed by ``print_empty_chapters``. + Returns: str: The chapters as a string with hidden chapters filtered out. """ + if self._super_chapters: + return self._to_string_with_super_chapters() + + return self._to_string_flat() + + def _to_string_flat(self) -> str: + """Render chapters without super-chapter grouping (original behaviour).""" result = "" for chapter in self._sorted_chapters(): # Skip hidden chapters from output @@ -196,6 +226,42 @@ def to_string(self) -> str: # Note: strip is required to remove leading newline chars when empty chapters are not printed option return result.strip() + def _to_string_with_super_chapters(self) -> str: + """Render chapters grouped under super-chapter headings.""" + result = "" + for sc in self._super_chapters: + # Collect record IDs whose labels match this super chapter + matching_ids: set[str] = set() + for rid, labels in self._record_labels.items(): + if any(lbl in sc.labels for lbl in labels): + matching_ids.add(rid) + + sc_block = "" + for chapter in self._sorted_chapters(): + if chapter.hidden: + continue + + # Filter chapter rows to only those matching the super chapter + filtered_rows = { + rid: row for rid, row in chapter.rows.items() if str(rid) in matching_ids or rid in matching_ids + } + if not filtered_rows and not self.print_empty_chapters: + continue + + sorted_items = sorted(filtered_rows.items(), key=lambda item: item[0], reverse=not self.sort_ascending) + if sorted_items: + ch_str = f"### {chapter.title}\n" + "".join(f"- {value}\n" for _, value in sorted_items) + sc_block += ch_str.strip() + "\n\n" + elif self.print_empty_chapters: + sc_block += f"### {chapter.title}\n{chapter.empty_message}\n\n" + + if sc_block.strip(): + result += f"## {sc.title}\n{sc_block}" + elif self.print_empty_chapters: + result += f"## {sc.title}\nNo entries detected.\n\n" + + return result.strip() + def from_yaml_array(self, chapters: list[dict[str, Any]]) -> "CustomChapters": # type: ignore[override] """ Populate the custom chapters from a list of dict objects. @@ -262,8 +328,46 @@ def from_yaml_array(self, chapters: list[dict[str, Any]]) -> "CustomChapters": parsed_order=parsed_order, ) + # Parse super-chapter definitions from action inputs + self._super_chapters = self._parse_super_chapters(ActionInputs.get_super_chapters()) + return self + @staticmethod + def _parse_super_chapters(raw_super_chapters: list[dict[str, Any]]) -> list[SuperChapter]: + """Parse super-chapter YAML definitions into SuperChapter instances. + + Parameters: + raw_super_chapters: Parsed YAML list of dicts with 'title' and 'label'/'labels'. + + Returns: + List of validated SuperChapter instances; invalid entries are skipped with a warning. + """ + result: list[SuperChapter] = [] + for entry in raw_super_chapters: + if not isinstance(entry, dict): + logger.warning("Skipping super-chapter definition with invalid type %s: %s", type(entry), entry) + continue + if "title" not in entry: + logger.warning("Skipping super-chapter without title key: %s", entry) + continue + title = entry["title"] + + raw_labels = entry.get("labels", entry.get("label")) + if raw_labels is None: + logger.warning("Super-chapter '%s' has no 'label' or 'labels' key; skipping", title) + continue + if isinstance(raw_labels, str): + raw_labels = [raw_labels] + normalized = _normalize_labels(raw_labels) + if not normalized: + logger.warning("Super-chapter '%s' labels definition empty after normalization; skipping", title) + continue + + result.append(SuperChapter(title=title, labels=normalized)) + logger.debug("Registered super-chapter '%s' with labels: %s", title, normalized) + return result + @staticmethod def _parse_bool_flag(title: str, raw: Any, key: str) -> bool: """Parse and validate a boolean flag from a chapter config entry. diff --git a/release_notes_generator/utils/constants.py b/release_notes_generator/utils/constants.py index 89693c24..1adebefd 100644 --- a/release_notes_generator/utils/constants.py +++ b/release_notes_generator/utils/constants.py @@ -24,6 +24,7 @@ TAG_NAME = "tag-name" FROM_TAG_NAME = "from-tag-name" CHAPTERS = "chapters" +SUPER_CHAPTERS = "super-chapters" DUPLICITY_SCOPE = "duplicity-scope" DUPLICITY_ICON = "duplicity-icon" PUBLISHED_AT = "published-at" diff --git a/tests/unit/conftest.py b/tests/unit/conftest.py index c924e4dd..eb028c66 100644 --- a/tests/unit/conftest.py +++ b/tests/unit/conftest.py @@ -34,8 +34,10 @@ from release_notes_generator.model.record.pull_request_record import PullRequestRecord from release_notes_generator.chapters.service_chapters import ServiceChapters from release_notes_generator.model.chapter import Chapter +from release_notes_generator.action_inputs import ActionInputs from release_notes_generator.chapters.custom_chapters import CustomChapters from release_notes_generator.model.record.sub_issue_record import SubIssueRecord +from release_notes_generator.utils.enums import DuplicityScopeEnum from release_notes_generator.utils.github_rate_limiter import GithubRateLimiter from release_notes_generator.utils.record_utils import get_id @@ -97,6 +99,32 @@ def custom_chapters_not_print_empty_chapters(): return chapters +def make_super_chapters_cc(mocker, chapters_yaml, super_chapters_yaml, print_empty=True): + mocker.patch( + "release_notes_generator.chapters.custom_chapters.ActionInputs.get_super_chapters", + return_value=super_chapters_yaml, + ) + mocker.patch( + "release_notes_generator.chapters.custom_chapters.ActionInputs.get_hierarchy", + return_value=False, + ) + mocker.patch( + "release_notes_generator.chapters.custom_chapters.ActionInputs.get_verbose", + return_value=False, + ) + mocker.patch( + "release_notes_generator.chapters.custom_chapters.ActionInputs.get_duplicity_scope", + return_value=DuplicityScopeEnum.BOTH, + ) + mocker.patch( + "release_notes_generator.chapters.custom_chapters.ActionInputs.get_skip_release_notes_labels", + return_value=["skip-release-notes"], + ) + cc = CustomChapters(print_empty_chapters=print_empty) + cc.from_yaml_array(chapters_yaml) + return cc + + # Fixtures for Service Chapters @pytest.fixture def service_chapters(): diff --git a/tests/unit/release_notes_generator/chapters/test_custom_chapters.py b/tests/unit/release_notes_generator/chapters/test_custom_chapters.py index ba731f98..b5c873ba 100644 --- a/tests/unit/release_notes_generator/chapters/test_custom_chapters.py +++ b/tests/unit/release_notes_generator/chapters/test_custom_chapters.py @@ -22,6 +22,7 @@ from release_notes_generator.model.record.record import Record from release_notes_generator.utils.enums import DuplicityScopeEnum from release_notes_generator.action_inputs import ActionInputs +from tests.unit.conftest import make_super_chapters_cc @pytest.fixture @@ -445,13 +446,6 @@ def test_populate_hidden_chapter_assigns_records(record_stub): def test_populate_hidden_chapter_no_duplicity_count(record_stub, mocker): - """ - Test that hidden chapters don't increment the duplicity counter. - - When a record is assigned to a hidden chapter, to_chapter_row should be called - with add_into_chapters=False to prevent incrementing the chapter presence count. - This ensures hidden chapters don't contribute to duplicity detection. - """ # Arrange cc = CustomChapters() cc.from_yaml_array([{"title": "Hidden", "labels": "bug", "hidden": True}]) @@ -468,13 +462,6 @@ def test_populate_hidden_chapter_no_duplicity_count(record_stub, mocker): def test_populate_visible_chapter_duplicity_count(record_stub, mocker): - """ - Test that visible (non-hidden) chapters increment the duplicity counter. - - When a record is assigned to a visible chapter, to_chapter_row should be called - with add_into_chapters=True to increment the chapter presence count. - This ensures visible chapters contribute to duplicity detection as expected. - """ # Arrange cc = CustomChapters() cc.from_yaml_array([{"title": "Visible", "labels": "bug", "hidden": False}]) @@ -533,12 +520,6 @@ def test_to_string_hidden_chapter_excluded(): def test_to_string_all_hidden_returns_empty(): - """ - Test that when all chapters are hidden, to_string returns empty string. - - This also verifies that hidden chapters are never shown even when - print_empty_chapters is True, since they are filtered out before rendering. - """ # Arrange cc = CustomChapters() cc.print_empty_chapters = True # Verify hidden chapters ignored even with this True @@ -1251,3 +1232,104 @@ def test_coh_chapter_not_populated_via_label_routing(hierarchy_record_stub, monk assert "org/repo#C1" in cc.chapters["New Features 🎉"].rows assert "org/repo#O1" in cc.chapters["Silent Live 🤫"].rows assert "org/repo#O1" not in cc.chapters["New Features 🎉"].rows + + +def test_super_chapters_no_super_chapters_renders_flat(mocker, record_stub): + """When no super chapters are defined, output is flat (### headings only).""" + # Arrange + cc = make_super_chapters_cc(mocker, [{"title": "Features", "label": "feature"}], []) + cc.populate({"org/repo#1": record_stub("org/repo#1", ["feature"])}) + # Act + output = cc.to_string() + # Assert + assert output.startswith("### Features") + for line in output.splitlines(): + assert not line.startswith("## ") + + +def test_super_chapters_records_grouped_under_super_chapter(mocker, record_stub): + """Records matching a super-chapter label appear nested under the super-chapter heading.""" + # Arrange + cc = make_super_chapters_cc( + mocker, + [ + {"title": "Enhancements", "label": "enhancement"}, + {"title": "Bugfixes", "label": "bug"}, + ], + [ + {"title": "Atum server", "label": "atum-server"}, + {"title": "Atum agent", "label": "atum-agent"}, + ], + ) + r1 = record_stub("org/repo#1", ["enhancement", "atum-server"]) + r2 = record_stub("org/repo#2", ["enhancement", "atum-agent"]) + r3 = record_stub("org/repo#3", ["bug", "atum-agent"]) + cc.populate({"org/repo#1": r1, "org/repo#2": r2, "org/repo#3": r3}) + # Act + output = cc.to_string() + # Assert + assert "## Atum server" in output + assert "## Atum agent" in output + server_section = output.split("## Atum agent")[0] + assert "### Enhancements" in server_section + assert "org/repo#1 row" in server_section + assert "org/repo#2 row" not in server_section + agent_section = output.split("## Atum agent")[1] + assert "### Enhancements" in agent_section + assert "org/repo#2 row" in agent_section + assert "### Bugfixes" in agent_section + assert "org/repo#3 row" in agent_section + + +def test_super_chapters_record_in_multiple_super_chapters(mocker, record_stub): + """A record with labels matching multiple super chapters appears in each.""" + # Arrange + cc = make_super_chapters_cc( + mocker, + [{"title": "Enhancements", "label": "enhancement"}], + [{"title": "Module A", "label": "mod-a"}, {"title": "Module B", "label": "mod-b"}], + ) + r1 = record_stub("org/repo#1", ["enhancement", "mod-a", "mod-b"]) + cc.populate({"org/repo#1": r1}) + # Act + output = cc.to_string() + # Assert + assert output.count("org/repo#1 row") == 2 + + +def test_super_chapters_empty_super_chapter_skipped_when_print_empty_false(mocker, record_stub): + """Super chapter with no matching records is omitted when print_empty_chapters=False.""" + # Arrange + cc = make_super_chapters_cc( + mocker, + [{"title": "Features", "label": "feature"}], + [{"title": "Module A", "label": "mod-a"}, {"title": "Module B", "label": "mod-b"}], + print_empty=False, + ) + r1 = record_stub("org/repo#1", ["feature", "mod-a"]) + cc.populate({"org/repo#1": r1}) + # Act + output = cc.to_string() + # Assert + assert "## Module A" in output + assert "## Module B" not in output + + +def test_super_chapters_parse_logs_and_skips_invalid(mocker): + """Invalid super-chapter entries are skipped with a warning.""" + # Arrange + cc = make_super_chapters_cc( + mocker, + [{"title": "Features", "label": "feature"}], + [ + "not-a-dict", + {"no-title": True}, + {"title": "Missing labels"}, + {"title": "Valid", "label": "ok"}, + ], + ) + # Act + output = cc.to_string() + # Assert - only the "Valid" super chapter survives + assert "## Valid" in output + assert sum(1 for line in output.splitlines() if line.startswith("## ") and not line.startswith("### ")) == 1 From 8895bc23dc343612c2337ef4e807d2a70528ac4a Mon Sep 17 00:00:00 2001 From: miroslavpojer Date: Thu, 2 Apr 2026 11:58:26 +0200 Subject: [PATCH 02/20] feat: implement super-chapters functionality with uncategorized fallback --- release_notes_generator/action_inputs.py | 24 +++--- .../chapters/custom_chapters.py | 78 +++++++++++++------ release_notes_generator/utils/constants.py | 3 + tests/unit/conftest.py | 14 +++- .../chapters/test_custom_chapters.py | 63 +++++++++++++++ .../test_action_inputs.py | 55 +++++++++++++ 6 files changed, 198 insertions(+), 39 deletions(-) diff --git a/release_notes_generator/action_inputs.py b/release_notes_generator/action_inputs.py index c5716eaa..4139e9fe 100644 --- a/release_notes_generator/action_inputs.py +++ b/release_notes_generator/action_inputs.py @@ -22,7 +22,6 @@ import os import sys import re - import yaml from release_notes_generator.utils.constants import ( @@ -167,7 +166,7 @@ def get_chapters() -> list[dict[str, str]]: logger.error("Error: 'chapters' input is not a valid YAML list.") return [] except yaml.YAMLError as exc: - logger.error("Error parsing 'chapters' input: {%s}", exc) + logger.error("Error parsing 'chapters' input: %s", exc) return [] return chapters @@ -176,25 +175,24 @@ def get_chapters() -> list[dict[str, str]]: def get_super_chapters() -> list[dict[str, str]]: """ Get list of super chapter definitions from the action inputs. - - Returns: - Parsed YAML list of dicts with 'title' and 'label'/'labels' keys, - or an empty list when the input is absent or invalid. """ - raw: str = get_action_input(SUPER_CHAPTERS, default="") # type: ignore[assignment] - if not raw or not raw.strip(): - return [] + # Get the 'super-chapters' input from environment variables + super_chapters_input: str = get_action_input(SUPER_CHAPTERS, default="") # type: ignore[assignment] + # mypy: string is returned as default + # Parse the received string back to YAML array input. try: - parsed = yaml.safe_load(raw) - if not isinstance(parsed, list): + super_chapters = yaml.safe_load(super_chapters_input) + if super_chapters is None: + return [] + if not isinstance(super_chapters, list): logger.error("Error: 'super-chapters' input is not a valid YAML list.") return [] except yaml.YAMLError as exc: - logger.error("Error parsing 'super-chapters' input: {%s}", exc) + logger.error("Error parsing 'super-chapters' input: %s", exc) return [] - return parsed + return super_chapters @staticmethod def get_hierarchy() -> bool: diff --git a/release_notes_generator/chapters/custom_chapters.py b/release_notes_generator/chapters/custom_chapters.py index b1f1a485..5cbac0f3 100644 --- a/release_notes_generator/chapters/custom_chapters.py +++ b/release_notes_generator/chapters/custom_chapters.py @@ -26,6 +26,7 @@ from release_notes_generator.action_inputs import ActionInputs from release_notes_generator.chapters.base_chapters import BaseChapters from release_notes_generator.model.chapter import Chapter +from release_notes_generator.utils.constants import UNCATEGORIZED_CHAPTER_TITLE from release_notes_generator.model.record.commit_record import CommitRecord from release_notes_generator.model.record.hierarchy_issue_record import HierarchyIssueRecord from release_notes_generator.model.record.record import Record @@ -149,10 +150,7 @@ def populate(self, records: dict[str, Record]) -> None: continue record_labels = getattr(record, "labels", []) - - # Track labels for super-chapter grouping at render time - if record_labels: - self._record_labels[record_id] = list(record_labels) + self._record_labels[record_id] = list(record_labels) # Conditional Custom Chapter gate: intercept open hierarchy parents before label routing. # Note: precedes the record_labels early-exit so label-less HierarchyIssueRecord @@ -226,40 +224,72 @@ def _to_string_flat(self) -> str: # Note: strip is required to remove leading newline chars when empty chapters are not printed option return result.strip() + 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(): + if any(lbl in sc.labels for lbl in labels): + matching.add(rid) + return matching + + def _render_chapter_for_ids(self, chapter: Chapter, matching_ids: set[str]) -> str: + """Render a single chapter filtered to matching_ids, delegating to Chapter.to_string.""" + original_rows = chapter.rows + chapter.rows = { + rid: row for rid, row in original_rows.items() if str(rid) in matching_ids or rid in matching_ids + } + try: + return chapter.to_string(sort_ascending=self.sort_ascending, print_empty_chapters=self.print_empty_chapters) + finally: + chapter.rows = original_rows + def _to_string_with_super_chapters(self) -> str: """Render chapters grouped under super-chapter headings.""" + # Collect all record IDs claimed by at least one super chapter + all_super_labels: set[str] = set() + for sc in self._super_chapters: + all_super_labels.update(sc.labels) + + claimed_ids: set[str] = set() + for rid, labels in self._record_labels.items(): + if any(lbl in all_super_labels for lbl in labels): + claimed_ids.add(rid) + result = "" for sc in self._super_chapters: - # Collect record IDs whose labels match this super chapter - matching_ids: set[str] = set() - for rid, labels in self._record_labels.items(): - if any(lbl in sc.labels for lbl in labels): - matching_ids.add(rid) + matching_ids = self._collect_super_chapter_ids(sc) sc_block = "" for chapter in self._sorted_chapters(): if chapter.hidden: continue - - # Filter chapter rows to only those matching the super chapter - filtered_rows = { - rid: row for rid, row in chapter.rows.items() if str(rid) in matching_ids or rid in matching_ids - } - if not filtered_rows and not self.print_empty_chapters: - continue - - sorted_items = sorted(filtered_rows.items(), key=lambda item: item[0], reverse=not self.sort_ascending) - if sorted_items: - ch_str = f"### {chapter.title}\n" + "".join(f"- {value}\n" for _, value in sorted_items) - sc_block += ch_str.strip() + "\n\n" - elif self.print_empty_chapters: - sc_block += f"### {chapter.title}\n{chapter.empty_message}\n\n" + ch_str = self._render_chapter_for_ids(chapter, matching_ids) + if ch_str: + sc_block += ch_str + "\n\n" if sc_block.strip(): result += f"## {sc.title}\n{sc_block}" elif self.print_empty_chapters: result += f"## {sc.title}\nNo entries detected.\n\n" + # Fallback: records not claimed by any super chapter + unclaimed_ids: set[str] = set() + for chapter in self._sorted_chapters(): + for rid in chapter.rows: + if str(rid) not in claimed_ids and rid not in claimed_ids: + unclaimed_ids.add(str(rid)) + + if unclaimed_ids: + uc_block = "" + for chapter in self._sorted_chapters(): + if chapter.hidden: + continue + ch_str = self._render_chapter_for_ids(chapter, unclaimed_ids) + if ch_str: + uc_block += ch_str + "\n\n" + if uc_block.strip(): + result += f"## {UNCATEGORIZED_CHAPTER_TITLE}\n{uc_block}" + return result.strip() def from_yaml_array(self, chapters: list[dict[str, Any]]) -> "CustomChapters": # type: ignore[override] @@ -334,7 +364,7 @@ def from_yaml_array(self, chapters: list[dict[str, Any]]) -> "CustomChapters": return self @staticmethod - def _parse_super_chapters(raw_super_chapters: list[dict[str, Any]]) -> list[SuperChapter]: + def _parse_super_chapters(raw_super_chapters: list[dict[str, str]]) -> list[SuperChapter]: """Parse super-chapter YAML definitions into SuperChapter instances. Parameters: diff --git a/release_notes_generator/utils/constants.py b/release_notes_generator/utils/constants.py index 1adebefd..df4e5970 100644 --- a/release_notes_generator/utils/constants.py +++ b/release_notes_generator/utils/constants.py @@ -49,6 +49,9 @@ SERVICE_CHAPTER_ORDER = "service-chapter-order" PRINT_EMPTY_CHAPTERS = "print-empty-chapters" +# Super chapter fallback heading +UNCATEGORIZED_CHAPTER_TITLE: str = "Uncategorized" + # Release notes comment constants RELEASE_NOTE_TITLE_DEFAULT: str = "[Rr]elease [Nn]otes:" CODERABBIT_RELEASE_NOTE_TITLE_DEFAULT: str = "Summary by CodeRabbit" diff --git a/tests/unit/conftest.py b/tests/unit/conftest.py index eb028c66..d13a6b51 100644 --- a/tests/unit/conftest.py +++ b/tests/unit/conftest.py @@ -34,6 +34,10 @@ from release_notes_generator.model.record.pull_request_record import PullRequestRecord from release_notes_generator.chapters.service_chapters import ServiceChapters from release_notes_generator.model.chapter import Chapter +from typing import Any + +from pytest_mock import MockerFixture + from release_notes_generator.action_inputs import ActionInputs from release_notes_generator.chapters.custom_chapters import CustomChapters from release_notes_generator.model.record.sub_issue_record import SubIssueRecord @@ -99,14 +103,20 @@ def custom_chapters_not_print_empty_chapters(): return chapters -def make_super_chapters_cc(mocker, chapters_yaml, super_chapters_yaml, print_empty=True): +def make_super_chapters_cc( + mocker: MockerFixture, + chapters_yaml: list[dict], + super_chapters_yaml: list[dict], + print_empty: bool = True, + hierarchy: bool = False, +) -> CustomChapters: mocker.patch( "release_notes_generator.chapters.custom_chapters.ActionInputs.get_super_chapters", return_value=super_chapters_yaml, ) mocker.patch( "release_notes_generator.chapters.custom_chapters.ActionInputs.get_hierarchy", - return_value=False, + return_value=hierarchy, ) mocker.patch( "release_notes_generator.chapters.custom_chapters.ActionInputs.get_verbose", diff --git a/tests/unit/release_notes_generator/chapters/test_custom_chapters.py b/tests/unit/release_notes_generator/chapters/test_custom_chapters.py index b5c873ba..2f53047d 100644 --- a/tests/unit/release_notes_generator/chapters/test_custom_chapters.py +++ b/tests/unit/release_notes_generator/chapters/test_custom_chapters.py @@ -1333,3 +1333,66 @@ def test_super_chapters_parse_logs_and_skips_invalid(mocker): # Assert - only the "Valid" super chapter survives assert "## Valid" in output assert sum(1 for line in output.splitlines() if line.startswith("## ") and not line.startswith("### ")) == 1 + + +def test_super_chapters_uncategorized_fallback(mocker, record_stub): + """Records without any super-chapter label appear under '## Uncategorized'.""" + # Arrange + cc = make_super_chapters_cc( + mocker, + [ + {"title": "Enhancements", "label": "enhancement"}, + {"title": "Bugfixes", "label": "bug"}, + ], + [{"title": "Module A", "label": "mod-a"}], + ) + r1 = record_stub("org/repo#1", ["enhancement", "mod-a"]) + r2 = record_stub("org/repo#2", ["bug"]) # no super-chapter label + cc.populate({"org/repo#1": r1, "org/repo#2": r2}) + # Act + output = cc.to_string() + # Assert + assert "## Module A" in output + assert "org/repo#1 row" in output + assert "## Uncategorized" in output + uncategorized_section = output.split("## Uncategorized")[1] + assert "org/repo#2 row" in uncategorized_section + assert "org/repo#1 row" not in uncategorized_section + + +def test_super_chapters_no_uncategorized_when_all_claimed(mocker, record_stub): + """No '## Uncategorized' section when all records match a super chapter.""" + # Arrange + cc = make_super_chapters_cc( + mocker, + [{"title": "Enhancements", "label": "enhancement"}], + [{"title": "Module A", "label": "mod-a"}], + ) + r1 = record_stub("org/repo#1", ["enhancement", "mod-a"]) + cc.populate({"org/repo#1": r1}) + # Act + output = cc.to_string() + # Assert + assert "## Module A" in output + assert "Uncategorized" not in output + + +def test_super_chapters_coh_record_visible_in_fallback(mocker, hierarchy_record_stub): + """COH-routed label-less record appears in Uncategorized when super chapters are active.""" + # Arrange + cc = make_super_chapters_cc( + mocker, + [ + {"title": "Features", "labels": "feature"}, + {"title": "Silent Live", "catch-open-hierarchy": True}, + ], + [{"title": "Module A", "label": "mod-a"}], + hierarchy=True, + ) + record = hierarchy_record_stub("org/repo#H1", [], state="open") + cc.populate({"org/repo#H1": record}) + # Act + output = cc.to_string() + # Assert - COH record appears in Uncategorized since it has no super-chapter label + assert "## Uncategorized" in output + assert "org/repo#H1 row" in output diff --git a/tests/unit/release_notes_generator/test_action_inputs.py b/tests/unit/release_notes_generator/test_action_inputs.py index d7c981dd..48bf3c1f 100644 --- a/tests/unit/release_notes_generator/test_action_inputs.py +++ b/tests/unit/release_notes_generator/test_action_inputs.py @@ -202,6 +202,61 @@ def test_get_chapters_yaml_error(mocker): assert [] == ActionInputs.get_chapters() +def test_get_super_chapters_success(mocker): + mocker.patch( + "release_notes_generator.action_inputs.get_action_input", + return_value='[{"title": "Module A", "label": "mod-a"}]', + ) + assert ActionInputs.get_super_chapters() == [{"title": "Module A", "label": "mod-a"}] + + +def test_get_super_chapters_empty_input(mocker): + mocker.patch("release_notes_generator.action_inputs.get_action_input", return_value="") + assert ActionInputs.get_super_chapters() == [] + + +def test_get_super_chapters_blank_input(mocker): + mocker.patch("release_notes_generator.action_inputs.get_action_input", return_value=" ") + assert ActionInputs.get_super_chapters() == [] + + +def test_get_super_chapters_not_a_list(mocker, caplog): + caplog.set_level("ERROR") + mocker.patch("release_notes_generator.action_inputs.get_action_input", return_value="not_a_list: true") + result = ActionInputs.get_super_chapters() + assert result == [] + assert any("not a valid YAML list" in r.message for r in caplog.records) + + +def test_get_super_chapters_yaml_error(mocker, caplog): + caplog.set_level("ERROR") + mocker.patch( + "release_notes_generator.action_inputs.get_action_input", + return_value='[{"title": "A" "label": "b"}]', + ) + result = ActionInputs.get_super_chapters() + assert result == [] + assert any("Error parsing 'super-chapters'" in r.message for r in caplog.records) + + +def test_get_super_chapters_list_labels_preserved(mocker): + mocker.patch( + "release_notes_generator.action_inputs.get_action_input", + return_value='[{"title": "Module A", "labels": ["mod-a", "mod-b"]}]', + ) + result = ActionInputs.get_super_chapters() + assert result == [{"title": "Module A", "labels": ["mod-a", "mod-b"]}] + + +def test_get_super_chapters_non_dict_entry_included(mocker): + mocker.patch( + "release_notes_generator.action_inputs.get_action_input", + return_value='["not-a-dict", {"title": "Valid", "label": "ok"}]', + ) + result = ActionInputs.get_super_chapters() + assert result == ["not-a-dict", {"title": "Valid", "label": "ok"}] + + def test_get_warnings(mocker): mocker.patch("release_notes_generator.action_inputs.get_action_input", return_value="true") assert ActionInputs.get_warnings() is True From b5b068c5d13af0c8ab8046ed04f71e00b4e82b4e Mon Sep 17 00:00:00 2001 From: miroslavpojer Date: Thu, 2 Apr 2026 12:17:43 +0200 Subject: [PATCH 03/20] fix: improve unclaimed IDs handling and normalize label input in CustomChapters --- release_notes_generator/chapters/custom_chapters.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/release_notes_generator/chapters/custom_chapters.py b/release_notes_generator/chapters/custom_chapters.py index 5cbac0f3..c2d837d2 100644 --- a/release_notes_generator/chapters/custom_chapters.py +++ b/release_notes_generator/chapters/custom_chapters.py @@ -275,9 +275,10 @@ def _to_string_with_super_chapters(self) -> str: # Fallback: records not claimed by any super chapter unclaimed_ids: set[str] = set() for chapter in self._sorted_chapters(): - for rid in chapter.rows: - if str(rid) not in claimed_ids and rid not in claimed_ids: - unclaimed_ids.add(str(rid)) + for row_id in chapter.rows: + row_id_str = str(row_id) + if row_id_str not in claimed_ids: + unclaimed_ids.add(row_id_str) if unclaimed_ids: uc_block = "" @@ -387,9 +388,8 @@ def _parse_super_chapters(raw_super_chapters: list[dict[str, str]]) -> list[Supe if raw_labels is None: logger.warning("Super-chapter '%s' has no 'label' or 'labels' key; skipping", title) continue - if isinstance(raw_labels, str): - raw_labels = [raw_labels] - normalized = _normalize_labels(raw_labels) + labels_input: str | list[str] = [raw_labels] if isinstance(raw_labels, str) else raw_labels + normalized = _normalize_labels(labels_input) if not normalized: logger.warning("Super-chapter '%s' labels definition empty after normalization; skipping", title) continue From d9291bc756b5211122675cc9218101c4207c5c24 Mon Sep 17 00:00:00 2001 From: miroslavpojer Date: Thu, 2 Apr 2026 13:25:55 +0200 Subject: [PATCH 04/20] Fixed review comments. --- docs/features/custom_chapters.md | 6 +- release_notes_generator/action_inputs.py | 44 +++++++++-- .../chapters/custom_chapters.py | 75 ++----------------- release_notes_generator/utils/utils.py | 39 +++++++++- tests/unit/conftest.py | 5 +- .../chapters/test_custom_chapters.py | 32 ++++---- .../test_action_inputs.py | 41 +++++++++- 7 files changed, 140 insertions(+), 102 deletions(-) diff --git a/docs/features/custom_chapters.md b/docs/features/custom_chapters.md index f6fdf9c0..489fd146 100644 --- a/docs/features/custom_chapters.md +++ b/docs/features/custom_chapters.md @@ -253,9 +253,9 @@ When super chapters are configured the output uses `##` headings for super chapt ``` ### Behavior -- A record is placed under a super chapter if it carries at least one label matching the super chapter's labels. -- A record can appear in **multiple** super chapters if its labels match more than one. -- Within each super chapter, records are routed to regular chapters by the normal label matching rules. +- A record is placed under a super-chapter if it carries at least one label matching the super-chapter's labels. +- A record can appear in **multiple** super-chapters if its labels match more than one. +- Within each super-chapter, records are routed to regular chapters by the normal label-matching rules. - Empty super chapters (no matching records) respect the `print-empty-chapters` setting. - When no super chapters are configured, output is flat (unchanged from previous behavior). diff --git a/release_notes_generator/action_inputs.py b/release_notes_generator/action_inputs.py index 4139e9fe..1a571e28 100644 --- a/release_notes_generator/action_inputs.py +++ b/release_notes_generator/action_inputs.py @@ -22,6 +22,7 @@ import os import sys import re +from typing import Any import yaml from release_notes_generator.utils.constants import ( @@ -58,7 +59,7 @@ ) from release_notes_generator.utils.enums import DuplicityScopeEnum from release_notes_generator.utils.gh_action import get_action_input -from release_notes_generator.utils.utils import normalize_version_tag +from release_notes_generator.utils.utils import normalize_labels, normalize_version_tag logger = logging.getLogger(__name__) @@ -172,9 +173,16 @@ def get_chapters() -> list[dict[str, str]]: return chapters @staticmethod - def get_super_chapters() -> list[dict[str, str]]: + def get_super_chapters() -> list[dict[str, Any]]: """ - Get list of super chapter definitions from the action inputs. + Get list of validated super-chapter definitions from the action inputs. + + Each returned entry is guaranteed to have: + - 'title': str + - 'labels': list[str] (non-empty, normalized) + + Invalid entries (non-dict, missing title, missing/empty labels) are skipped + with a warning log. """ # Get the 'super-chapters' input from environment variables super_chapters_input: str = get_action_input(SUPER_CHAPTERS, default="") # type: ignore[assignment] @@ -182,17 +190,39 @@ def get_super_chapters() -> list[dict[str, str]]: # Parse the received string back to YAML array input. try: - super_chapters = yaml.safe_load(super_chapters_input) - if super_chapters is None: + raw_list = yaml.safe_load(super_chapters_input) + if raw_list is None: return [] - if not isinstance(super_chapters, list): + if not isinstance(raw_list, list): logger.error("Error: 'super-chapters' input is not a valid YAML list.") return [] except yaml.YAMLError as exc: logger.error("Error parsing 'super-chapters' input: %s", exc) return [] - return super_chapters + result: list[dict[str, Any]] = [] + for entry in raw_list: + if not isinstance(entry, dict): + logger.warning("Skipping super-chapter definition with invalid type %s: %s", type(entry), entry) + continue + if "title" not in entry: + logger.warning("Skipping super-chapter without title key: %s", entry) + continue + title = entry["title"] + + raw_labels = entry.get("labels", entry.get("label")) + if raw_labels is None: + logger.warning("Super-chapter '%s' has no 'label' or 'labels' key; skipping", title) + continue + labels_input: str | list[str] = [raw_labels] if isinstance(raw_labels, str) else raw_labels + normalized = normalize_labels(labels_input) + if not normalized: + logger.warning("Super-chapter '%s' labels definition empty after normalization; skipping", title) + continue + + result.append({"title": title, "labels": normalized}) + logger.debug("Validated super-chapter '%s' with labels: %s", title, normalized) + return result @staticmethod def get_hierarchy() -> bool: diff --git a/release_notes_generator/chapters/custom_chapters.py b/release_notes_generator/chapters/custom_chapters.py index c2d837d2..e9baa4bb 100644 --- a/release_notes_generator/chapters/custom_chapters.py +++ b/release_notes_generator/chapters/custom_chapters.py @@ -21,7 +21,7 @@ import logging from dataclasses import dataclass, field -from typing import Any, Iterable +from typing import Any from release_notes_generator.action_inputs import ActionInputs from release_notes_generator.chapters.base_chapters import BaseChapters @@ -30,6 +30,7 @@ from release_notes_generator.model.record.commit_record import CommitRecord from release_notes_generator.model.record.hierarchy_issue_record import HierarchyIssueRecord from release_notes_generator.model.record.record import Record +from release_notes_generator.utils.utils import normalize_labels logger = logging.getLogger(__name__) @@ -42,44 +43,6 @@ class SuperChapter: labels: list[str] = field(default_factory=list) -def _normalize_labels(raw: Any) -> list[str]: # helper for multi-label - """Normalize a raw labels definition into an ordered, de-duplicated list. - - Parameters: - - str: may contain newlines and/or commas - - list[str]: already a sequence of labels (will still be trimmed & deduped) - - Returns: - Ordered list preserving first occurrence order; excludes empty tokens. - Invalid (non str/list) returns empty list to be handled by caller. - """ - if isinstance(raw, list): - working: Iterable[Any] = raw - elif isinstance(raw, str): - # newline first then comma per spec - parts: list[str] = [] - for line in raw.splitlines(): - for token in line.split(","): - parts.append(token) - working = parts - else: - return [] - - cleaned: list[str] = [] - seen: set[str] = set() - for item in working: - if not isinstance(item, str): # skip non-string items silently - continue - token = item.strip() - if not token: # skip empty after trimming - continue - if token in seen: # de-duplicate preserving first occurrence - continue - seen.add(token) - cleaned.append(token) - return cleaned - - class CustomChapters(BaseChapters): """ A class used to represent the custom chapters in the release notes. @@ -365,38 +328,16 @@ def from_yaml_array(self, chapters: list[dict[str, Any]]) -> "CustomChapters": return self @staticmethod - def _parse_super_chapters(raw_super_chapters: list[dict[str, str]]) -> list[SuperChapter]: - """Parse super-chapter YAML definitions into SuperChapter instances. + def _parse_super_chapters(validated: list[dict[str, Any]]) -> list[SuperChapter]: + """Construct SuperChapter instances from pre-validated ActionInputs dicts. Parameters: - raw_super_chapters: Parsed YAML list of dicts with 'title' and 'label'/'labels'. + validated: List of fully-validated super-chapter dicts with 'title' and 'labels' keys. Returns: - List of validated SuperChapter instances; invalid entries are skipped with a warning. + List of SuperChapter instances. """ - result: list[SuperChapter] = [] - for entry in raw_super_chapters: - if not isinstance(entry, dict): - logger.warning("Skipping super-chapter definition with invalid type %s: %s", type(entry), entry) - continue - if "title" not in entry: - logger.warning("Skipping super-chapter without title key: %s", entry) - continue - title = entry["title"] - - raw_labels = entry.get("labels", entry.get("label")) - if raw_labels is None: - logger.warning("Super-chapter '%s' has no 'label' or 'labels' key; skipping", title) - continue - labels_input: str | list[str] = [raw_labels] if isinstance(raw_labels, str) else raw_labels - normalized = _normalize_labels(labels_input) - if not normalized: - logger.warning("Super-chapter '%s' labels definition empty after normalization; skipping", title) - continue - - result.append(SuperChapter(title=title, labels=normalized)) - logger.debug("Registered super-chapter '%s' with labels: %s", title, normalized) - return result + return [SuperChapter(title=e["title"], labels=e["labels"]) for e in validated] @staticmethod def _parse_bool_flag(title: str, raw: Any, key: str) -> bool: @@ -504,7 +445,7 @@ def _parse_labels_definition(title: str, chapter: dict[str, Any], catch_open_hie ) return None - normalized = _normalize_labels(raw_labels) + normalized = normalize_labels(raw_labels) if not normalized: logger.warning("Chapter '%s' labels definition empty after normalization; skipping", title) return None diff --git a/release_notes_generator/utils/utils.py b/release_notes_generator/utils/utils.py index 179a7894..c3285346 100644 --- a/release_notes_generator/utils/utils.py +++ b/release_notes_generator/utils/utils.py @@ -21,7 +21,8 @@ import logging import re -from typing import Optional +from collections.abc import Iterable +from typing import Any, Optional from github.GitRelease import GitRelease from github.Repository import Repository @@ -29,6 +30,42 @@ logger = logging.getLogger(__name__) +def normalize_labels(raw: Any) -> list[str]: + """Normalize a raw labels definition into an ordered, de-duplicated list. + + Parameters: + raw: May be a str (newline/comma separated) or list[str]. Any other type returns []. + + Returns: + Ordered list preserving first-occurrence order; excludes empty tokens. + Invalid (non str/list) input returns an empty list. + """ + if isinstance(raw, list): + working: Iterable[Any] = raw + elif isinstance(raw, str): + parts: list[str] = [] + for line in raw.splitlines(): + for token in line.split(","): + parts.append(token) + working = parts + else: + return [] + + cleaned: list[str] = [] + seen: set[str] = set() + for item in working: + if not isinstance(item, str): + continue + token = item.strip() + if not token: + continue + if token in seen: + continue + seen.add(token) + cleaned.append(token) + return cleaned + + def get_change_url( tag_name: str, repository: Optional[Repository] = None, git_release: Optional[GitRelease] = None ) -> str: diff --git a/tests/unit/conftest.py b/tests/unit/conftest.py index d13a6b51..58ad2d19 100644 --- a/tests/unit/conftest.py +++ b/tests/unit/conftest.py @@ -38,7 +38,6 @@ from pytest_mock import MockerFixture -from release_notes_generator.action_inputs import ActionInputs from release_notes_generator.chapters.custom_chapters import CustomChapters from release_notes_generator.model.record.sub_issue_record import SubIssueRecord from release_notes_generator.utils.enums import DuplicityScopeEnum @@ -105,8 +104,8 @@ def custom_chapters_not_print_empty_chapters(): def make_super_chapters_cc( mocker: MockerFixture, - chapters_yaml: list[dict], - super_chapters_yaml: list[dict], + chapters_yaml: list[dict[str, Any]], + super_chapters_yaml: list[Any], print_empty: bool = True, hierarchy: bool = False, ) -> CustomChapters: diff --git a/tests/unit/release_notes_generator/chapters/test_custom_chapters.py b/tests/unit/release_notes_generator/chapters/test_custom_chapters.py index 2f53047d..de3b1cf2 100644 --- a/tests/unit/release_notes_generator/chapters/test_custom_chapters.py +++ b/tests/unit/release_notes_generator/chapters/test_custom_chapters.py @@ -15,7 +15,8 @@ # import pytest -from release_notes_generator.chapters.custom_chapters import CustomChapters, _normalize_labels +from release_notes_generator.chapters.custom_chapters import CustomChapters +from release_notes_generator.utils.utils import normalize_labels from release_notes_generator.model.record.issue_record import IssueRecord from release_notes_generator.model.record.hierarchy_issue_record import HierarchyIssueRecord from release_notes_generator.model.record.commit_record import CommitRecord @@ -302,7 +303,7 @@ def test_from_yaml_array_verbose_debug_branch(monkeypatch, caplog): ) def test_normalize_labels_edge_cases(raw, expected): # Arrange / Act - result = _normalize_labels(raw) + result = normalize_labels(raw) # Assert assert result == expected @@ -1257,8 +1258,8 @@ def test_super_chapters_records_grouped_under_super_chapter(mocker, record_stub) {"title": "Bugfixes", "label": "bug"}, ], [ - {"title": "Atum server", "label": "atum-server"}, - {"title": "Atum agent", "label": "atum-agent"}, + {"title": "Atum server", "labels": ["atum-server"]}, + {"title": "Atum agent", "labels": ["atum-agent"]}, ], ) r1 = record_stub("org/repo#1", ["enhancement", "atum-server"]) @@ -1287,7 +1288,7 @@ def test_super_chapters_record_in_multiple_super_chapters(mocker, record_stub): cc = make_super_chapters_cc( mocker, [{"title": "Enhancements", "label": "enhancement"}], - [{"title": "Module A", "label": "mod-a"}, {"title": "Module B", "label": "mod-b"}], + [{"title": "Module A", "labels": ["mod-a"]}, {"title": "Module B", "labels": ["mod-b"]}], ) r1 = record_stub("org/repo#1", ["enhancement", "mod-a", "mod-b"]) cc.populate({"org/repo#1": r1}) @@ -1303,7 +1304,7 @@ def test_super_chapters_empty_super_chapter_skipped_when_print_empty_false(mocke cc = make_super_chapters_cc( mocker, [{"title": "Features", "label": "feature"}], - [{"title": "Module A", "label": "mod-a"}, {"title": "Module B", "label": "mod-b"}], + [{"title": "Module A", "labels": ["mod-a"]}, {"title": "Module B", "labels": ["mod-b"]}], print_empty=False, ) r1 = record_stub("org/repo#1", ["feature", "mod-a"]) @@ -1316,21 +1317,16 @@ def test_super_chapters_empty_super_chapter_skipped_when_print_empty_false(mocke def test_super_chapters_parse_logs_and_skips_invalid(mocker): - """Invalid super-chapter entries are skipped with a warning.""" - # Arrange + """Only valid super-chapter entries produce headings; validation now runs in ActionInputs.""" + # Arrange - mock returns only the one valid entry (as ActionInputs would after validation) cc = make_super_chapters_cc( mocker, [{"title": "Features", "label": "feature"}], - [ - "not-a-dict", - {"no-title": True}, - {"title": "Missing labels"}, - {"title": "Valid", "label": "ok"}, - ], + [{"title": "Valid", "labels": ["ok"]}], ) # Act output = cc.to_string() - # Assert - only the "Valid" super chapter survives + # Assert - only the "Valid" super chapter produces a heading assert "## Valid" in output assert sum(1 for line in output.splitlines() if line.startswith("## ") and not line.startswith("### ")) == 1 @@ -1344,7 +1340,7 @@ def test_super_chapters_uncategorized_fallback(mocker, record_stub): {"title": "Enhancements", "label": "enhancement"}, {"title": "Bugfixes", "label": "bug"}, ], - [{"title": "Module A", "label": "mod-a"}], + [{"title": "Module A", "labels": ["mod-a"]}], ) r1 = record_stub("org/repo#1", ["enhancement", "mod-a"]) r2 = record_stub("org/repo#2", ["bug"]) # no super-chapter label @@ -1366,7 +1362,7 @@ def test_super_chapters_no_uncategorized_when_all_claimed(mocker, record_stub): cc = make_super_chapters_cc( mocker, [{"title": "Enhancements", "label": "enhancement"}], - [{"title": "Module A", "label": "mod-a"}], + [{"title": "Module A", "labels": ["mod-a"]}], ) r1 = record_stub("org/repo#1", ["enhancement", "mod-a"]) cc.populate({"org/repo#1": r1}) @@ -1386,7 +1382,7 @@ def test_super_chapters_coh_record_visible_in_fallback(mocker, hierarchy_record_ {"title": "Features", "labels": "feature"}, {"title": "Silent Live", "catch-open-hierarchy": True}, ], - [{"title": "Module A", "label": "mod-a"}], + [{"title": "Module A", "labels": ["mod-a"]}], hierarchy=True, ) record = hierarchy_record_stub("org/repo#H1", [], state="open") diff --git a/tests/unit/release_notes_generator/test_action_inputs.py b/tests/unit/release_notes_generator/test_action_inputs.py index 48bf3c1f..439d3af6 100644 --- a/tests/unit/release_notes_generator/test_action_inputs.py +++ b/tests/unit/release_notes_generator/test_action_inputs.py @@ -207,7 +207,7 @@ def test_get_super_chapters_success(mocker): "release_notes_generator.action_inputs.get_action_input", return_value='[{"title": "Module A", "label": "mod-a"}]', ) - assert ActionInputs.get_super_chapters() == [{"title": "Module A", "label": "mod-a"}] + assert ActionInputs.get_super_chapters() == [{"title": "Module A", "labels": ["mod-a"]}] def test_get_super_chapters_empty_input(mocker): @@ -248,13 +248,48 @@ def test_get_super_chapters_list_labels_preserved(mocker): assert result == [{"title": "Module A", "labels": ["mod-a", "mod-b"]}] -def test_get_super_chapters_non_dict_entry_included(mocker): +def test_get_super_chapters_non_dict_entry_skipped(mocker, caplog): + caplog.set_level("WARNING") mocker.patch( "release_notes_generator.action_inputs.get_action_input", return_value='["not-a-dict", {"title": "Valid", "label": "ok"}]', ) result = ActionInputs.get_super_chapters() - assert result == ["not-a-dict", {"title": "Valid", "label": "ok"}] + assert result == [{"title": "Valid", "labels": ["ok"]}] + assert any("invalid type" in r.message for r in caplog.records) + + +def test_get_super_chapters_missing_title_skipped(mocker, caplog): + caplog.set_level("WARNING") + mocker.patch( + "release_notes_generator.action_inputs.get_action_input", + return_value='[{"no-title": true}, {"title": "Valid", "label": "v"}]', + ) + result = ActionInputs.get_super_chapters() + assert result == [{"title": "Valid", "labels": ["v"]}] + assert any("without title key" in r.message for r in caplog.records) + + +def test_get_super_chapters_missing_labels_skipped(mocker, caplog): + caplog.set_level("WARNING") + mocker.patch( + "release_notes_generator.action_inputs.get_action_input", + return_value='[{"title": "No labels"}, {"title": "Valid", "label": "v"}]', + ) + result = ActionInputs.get_super_chapters() + assert result == [{"title": "Valid", "labels": ["v"]}] + assert any("has no 'label' or 'labels' key" in r.message for r in caplog.records) + + +def test_get_super_chapters_empty_labels_after_normalization_skipped(mocker, caplog): + caplog.set_level("WARNING") + mocker.patch( + "release_notes_generator.action_inputs.get_action_input", + return_value='[{"title": "Empty", "labels": []}, {"title": "Valid", "label": "v"}]', + ) + result = ActionInputs.get_super_chapters() + assert result == [{"title": "Valid", "labels": ["v"]}] + assert any("empty after normalization" in r.message for r in caplog.records) def test_get_warnings(mocker): From d0250fd54bd7370c4bdef68a441dda80cab22281 Mon Sep 17 00:00:00 2001 From: miroslavpojer Date: Thu, 2 Apr 2026 13:34:24 +0200 Subject: [PATCH 05/20] Fix review comments. --- release_notes_generator/action_inputs.py | 3 +++ .../test_action_inputs.py | 22 +++++++++++++++++++ 2 files changed, 25 insertions(+) diff --git a/release_notes_generator/action_inputs.py b/release_notes_generator/action_inputs.py index 1a571e28..0a2ffa42 100644 --- a/release_notes_generator/action_inputs.py +++ b/release_notes_generator/action_inputs.py @@ -209,6 +209,9 @@ def get_super_chapters() -> list[dict[str, Any]]: logger.warning("Skipping super-chapter without title key: %s", entry) continue title = entry["title"] + if not isinstance(title, str) or not title.strip(): + logger.warning("Skipping super-chapter with invalid title value: %r", title) + continue raw_labels = entry.get("labels", entry.get("label")) if raw_labels is None: diff --git a/tests/unit/release_notes_generator/test_action_inputs.py b/tests/unit/release_notes_generator/test_action_inputs.py index 439d3af6..1bce9cc8 100644 --- a/tests/unit/release_notes_generator/test_action_inputs.py +++ b/tests/unit/release_notes_generator/test_action_inputs.py @@ -270,6 +270,28 @@ def test_get_super_chapters_missing_title_skipped(mocker, caplog): assert any("without title key" in r.message for r in caplog.records) +def test_get_super_chapters_non_string_title_skipped(mocker, caplog): + caplog.set_level("WARNING") + mocker.patch( + "release_notes_generator.action_inputs.get_action_input", + return_value='[{"title": 42, "label": "l"}, {"title": "Valid", "label": "v"}]', + ) + result = ActionInputs.get_super_chapters() + assert result == [{"title": "Valid", "labels": ["v"]}] + assert any("invalid title value" in r.message for r in caplog.records) + + +def test_get_super_chapters_blank_title_skipped(mocker, caplog): + caplog.set_level("WARNING") + mocker.patch( + "release_notes_generator.action_inputs.get_action_input", + return_value='[{"title": " ", "label": "l"}, {"title": "Valid", "label": "v"}]', + ) + result = ActionInputs.get_super_chapters() + assert result == [{"title": "Valid", "labels": ["v"]}] + assert any("invalid title value" in r.message for r in caplog.records) + + def test_get_super_chapters_missing_labels_skipped(mocker, caplog): caplog.set_level("WARNING") mocker.patch( From 8679c2a5ba457f394bd3fccedb792d06bc54c8e3 Mon Sep 17 00:00:00 2001 From: miroslavpojer Date: Thu, 2 Apr 2026 16:33:27 +0200 Subject: [PATCH 06/20] feat: enhance hierarchy issue rendering logic for open and closed parents (#280) * feat: enhance hierarchy issue rendering logic for open and closed parents * feat: add support for highlighting open sub-issues under closed hierarchy parents --- action.yml | 5 + docs/features/issue_hierarchy_support.md | 14 +- release_notes_generator/action_inputs.py | 48 ++--- .../model/record/hierarchy_issue_record.py | 44 ++-- release_notes_generator/utils/constants.py | 1 + release_notes_generator/utils/gh_action.py | 6 +- tests/unit/conftest.py | 106 ++++++++++ .../model/test_hierarchy_issue_record.py | 190 ++++++++++++++++-- .../utils/test_gh_action.py | 8 +- 9 files changed, 355 insertions(+), 67 deletions(-) diff --git a/action.yml b/action.yml index 674b3e20..a3a6edd8 100644 --- a/action.yml +++ b/action.yml @@ -53,6 +53,10 @@ inputs: description: 'Icon to be used for duplicity warning. Icon is placed before the record line.' required: false default: '🔔' + open-hierarchy-sub-issue-icon: + description: 'Icon prepended to open children under a closed hierarchy parent.' + required: false + default: '🟡' published-at: description: 'Use `published-at` timestamp instead of `created-at` as the reference point of previous Release.' required: false @@ -190,6 +194,7 @@ runs: INPUT_HIERARCHY: ${{ inputs.hierarchy }} INPUT_DUPLICITY_SCOPE: ${{ inputs.duplicity-scope }} INPUT_DUPLICITY_ICON: ${{ inputs.duplicity-icon }} + INPUT_OPEN_HIERARCHY_SUB_ISSUE_ICON: ${{ inputs.open-hierarchy-sub-issue-icon }} INPUT_WARNINGS: ${{ inputs.warnings }} INPUT_HIDDEN_SERVICE_CHAPTERS: ${{ inputs.hidden-service-chapters }} INPUT_SERVICE_CHAPTER_ORDER: ${{ inputs.service-chapter-order }} diff --git a/docs/features/issue_hierarchy_support.md b/docs/features/issue_hierarchy_support.md index 75475bcd..9cf6efb3 100644 --- a/docs/features/issue_hierarchy_support.md +++ b/docs/features/issue_hierarchy_support.md @@ -6,7 +6,12 @@ Represent issue → sub-issue relationships directly in release notes, aggregati ## How It Works - Enabled via input `hierarchy: true` (default: `false`). When disabled, all issues render flat. - Parent issues are detected; sub-issues (and nested hierarchy issues) are fetched and ordered by level. Levels indent with two spaces per depth; nested items use list markers (`-`). -- Only closed sub-issues that contain a change increment (merged PR to default branch) are rendered; open ones, and PR to non default branch are ignored. +- When the parent hierarchy issue is **open**: + - Leaf sub-issues: only closed ones with a change increment are rendered; open ones and those closed or delivered in a previous release are ignored. + - Nested sub-hierarchy children: filtered by change increment only — an open child that aggregates PRs from deeper levels is still rendered. +- When the parent hierarchy issue is **closed**: + - All children (sub-issues and nested sub-hierarchy children) are rendered. Open children are prefixed with the `open-hierarchy-sub-issue-icon` (default `🟡`) to signal incomplete work. + - Set `open-hierarchy-sub-issue-icon` to an empty string to disable the highlighting. - Each hierarchy issue line can expand with its own extracted release notes block if present (prefixed with `_Release Notes_:` heading within the item block). ## Configuration @@ -23,6 +28,12 @@ Represent issue → sub-issue relationships directly in release notes, aggregati - {"title": "New Features 🎉", "label": "feature"} ``` +Optional inputs related to hierarchy: + +| Input | Default | Description | +|---|---|---| +| `open-hierarchy-sub-issue-icon` | `🟡` | Icon prepended to open sub-issues and open sub-hierarchy issues rendered under a closed hierarchy parent. Set to an empty string to disable highlighting. | + ## Example Result ```markdown ### New Features 🎉 @@ -33,6 +44,7 @@ Represent issue → sub-issue relationships directly in release notes, aggregati - Updated `scala213 = "2.13.13"` - Feature: _Add user MFA enrollment flow_ #123 developed by @alice in #124 - Add user MFA enrollment flow + - 🟡 Feature: _Add OAuth2 login_ #125 ← open sub-issue under closed Epic ``` (1st four indented bullets under Epic line represent the extracted release notes from the parent hierarchy issue's body.) diff --git a/release_notes_generator/action_inputs.py b/release_notes_generator/action_inputs.py index 0a2ffa42..c0a7e316 100644 --- a/release_notes_generator/action_inputs.py +++ b/release_notes_generator/action_inputs.py @@ -40,6 +40,7 @@ PRINT_EMPTY_CHAPTERS, DUPLICITY_SCOPE, DUPLICITY_ICON, + OPEN_HIERARCHY_SUB_ISSUE_ICON, ROW_FORMAT_LINK_PR, ROW_FORMAT_ISSUE, ROW_FORMAT_PR, @@ -157,8 +158,7 @@ def get_chapters() -> list[dict[str, str]]: Get list of the chapters from the action inputs. Each chapter is a dict. """ # Get the 'chapters' input from environment variables - chapters_input: str = get_action_input(CHAPTERS, default="") # type: ignore[assignment] - # mypy: string is returned as default + chapters_input: str = get_action_input(CHAPTERS, default="") # Parse the received string back to YAML array input. try: @@ -240,8 +240,7 @@ def get_duplicity_scope() -> DuplicityScopeEnum: """ Get the duplicity scope parameter value from the action inputs. """ - duplicity_scope = get_action_input(DUPLICITY_SCOPE, "both").upper() # type: ignore[union-attr] - # mypy: string is returned as default + duplicity_scope = get_action_input(DUPLICITY_SCOPE, "both").upper() try: return DuplicityScopeEnum(duplicity_scope) @@ -254,15 +253,21 @@ def get_duplicity_icon() -> str: """ Get the duplicity icon from the action inputs. """ - return get_action_input(DUPLICITY_ICON, "🔔") # type: ignore[return-value] # string is returned as default + return get_action_input(DUPLICITY_ICON, "🔔") + + @staticmethod + def get_open_hierarchy_sub_issue_icon() -> str: + """ + Get the icon prepended to open sub-issues rendered under a closed hierarchy parent. + """ + return get_action_input(OPEN_HIERARCHY_SUB_ISSUE_ICON, "🟡") @staticmethod def get_published_at() -> bool: """ Get the published at parameter value from the action inputs. """ - return get_action_input(PUBLISHED_AT, "false").lower() == "true" # type: ignore[union-attr] - # mypy: string is returned as default + return get_action_input(PUBLISHED_AT, "false").lower() == "true" @staticmethod def get_skip_release_notes_labels() -> list[str]: @@ -281,35 +286,31 @@ def get_verbose() -> bool: Get the verbose parameter value from the action inputs. Safe for non-GitHub test contexts where the input may be unset (returns False by default). """ - raw = get_action_input(VERBOSE, "false") # type: ignore[assignment] + raw = get_action_input(VERBOSE, "false") # Some test contexts (unit/integration) do not populate GitHub inputs; fall back to default. raw_normalized = (raw or "false").strip().lower() return os.getenv(RUNNER_DEBUG, "0") == "1" or raw_normalized == "true" - # mypy: string is returned as default @staticmethod def get_release_notes_title() -> str: """ Get the release notes title from the action inputs. """ - return get_action_input(RELEASE_NOTES_TITLE, RELEASE_NOTE_TITLE_DEFAULT) # type: ignore[return-value] - # mypy: string is returned as default + return get_action_input(RELEASE_NOTES_TITLE, RELEASE_NOTE_TITLE_DEFAULT) @staticmethod def is_coderabbit_support_active() -> bool: """ Get the CodeRabbit support active parameter value from the action inputs. """ - return get_action_input(CODERABBIT_SUPPORT_ACTIVE, "false").lower() == "true" # type: ignore[union-attr] + return get_action_input(CODERABBIT_SUPPORT_ACTIVE, "false").lower() == "true" @staticmethod def get_coderabbit_release_notes_title() -> str: """ Get the CodeRabbit release notes title from the action inputs. """ - return get_action_input( - CODERABBIT_RELEASE_NOTES_TITLE, CODERABBIT_RELEASE_NOTE_TITLE_DEFAULT - ) # type: ignore[return-value] + return get_action_input(CODERABBIT_RELEASE_NOTES_TITLE, CODERABBIT_RELEASE_NOTE_TITLE_DEFAULT) @staticmethod def get_coderabbit_summary_ignore_groups() -> list[str]: @@ -335,7 +336,7 @@ def get_warnings() -> bool: """ Get the warnings parameter value from the action inputs. """ - return get_action_input(WARNINGS, "true").lower() == "true" # type: ignore[union-attr] + return get_action_input(WARNINGS, "true").lower() == "true" # mypy: string is returned as default @staticmethod @@ -433,9 +434,7 @@ def get_row_format_hierarchy_issue() -> str: """ if ActionInputs._row_format_hierarchy_issue is None: ActionInputs._row_format_hierarchy_issue = ActionInputs._detect_row_format_invalid_keywords( - get_action_input( - ROW_FORMAT_HIERARCHY_ISSUE, "{type}: _{title}_ {number}" - ).strip(), # type: ignore[union-attr] + get_action_input(ROW_FORMAT_HIERARCHY_ISSUE, "{type}: _{title}_ {number}").strip(), row_type=ActionInputs.ROW_TYPE_HIERARCHY_ISSUE, clean=True, ) @@ -450,9 +449,8 @@ def get_row_format_issue() -> str: ActionInputs._row_format_issue = ActionInputs._detect_row_format_invalid_keywords( get_action_input( ROW_FORMAT_ISSUE, "{type}: {number} _{title}_ developed by {developers} in {pull-requests}" - ).strip(), # type: ignore[union-attr] + ).strip(), clean=True, - # mypy: string is returned as default ) return ActionInputs._row_format_issue @@ -463,12 +461,9 @@ def get_row_format_pr() -> str: """ if ActionInputs._row_format_pr is None: ActionInputs._row_format_pr = ActionInputs._detect_row_format_invalid_keywords( - get_action_input( - ROW_FORMAT_PR, "{number} _{title}_ developed by {developers}" - ).strip(), # type: ignore[union-attr] + get_action_input(ROW_FORMAT_PR, "{number} _{title}_ developed by {developers}").strip(), row_type=ActionInputs.ROW_TYPE_PR, clean=True, - # mypy: string is returned as default ) return ActionInputs._row_format_pr @@ -477,8 +472,7 @@ def get_row_format_link_pr() -> bool: """ Get the value controlling whether the row format should include a 'PR:' prefix when linking to PRs. """ - return get_action_input(ROW_FORMAT_LINK_PR, "true").lower() == "true" # type: ignore[union-attr] - # mypy: string is returned as default + return get_action_input(ROW_FORMAT_LINK_PR, "true").lower() == "true" @staticmethod def validate_inputs() -> None: diff --git a/release_notes_generator/model/record/hierarchy_issue_record.py b/release_notes_generator/model/record/hierarchy_issue_record.py index 05a5aa4c..9b429584 100644 --- a/release_notes_generator/model/record/hierarchy_issue_record.py +++ b/release_notes_generator/model/record/hierarchy_issue_record.py @@ -216,24 +216,42 @@ def to_chapter_row(self, add_into_chapters: bool = True) -> str: # add sub-hierarchy issues for sub_hierarchy_issue in self._sub_hierarchy_issues.values(): - logger.debug("Rendering hierarchy issue row for sub-issue #%s", sub_hierarchy_issue.issue.number) - if sub_hierarchy_issue.contains_change_increment(): - logger.debug("Sub-hierarchy issue #%s contains change increment", sub_hierarchy_issue.issue.number) - row = f"{row}\n{sub_hierarchy_issue.to_chapter_row()}" + logger.debug("Rendering sub-hierarchy issue row for #%s", sub_hierarchy_issue.issue.number) + if self.is_open: + if not sub_hierarchy_issue.contains_change_increment(): + continue + # Closed parent: render all sub-hierarchy issues regardless of state or change increment + logger.debug("Rendering sub-hierarchy issue #%s", sub_hierarchy_issue.issue.number) + if self.is_closed and sub_hierarchy_issue.is_open: + sub_row = sub_hierarchy_issue.to_chapter_row() + # Highlight open children under a closed parent to signal incomplete work + icon = ActionInputs.get_open_hierarchy_sub_issue_icon() + header_line, newline, remaining_lines = sub_row.partition("\n") + header_text = header_line.lstrip() + indent = header_line[: len(header_line) - len(header_text)] + sub_row = f"{indent}{icon} {header_text}{newline}{remaining_lines}" + else: + sub_row = sub_hierarchy_issue.to_chapter_row() + row = f"{row}\n{sub_row}" # add sub-issues if len(self._sub_issues) > 0: sub_indent = " " * (self._level + 1) for sub_issue in self._sub_issues.values(): - logger.debug("Rendering sub-issue row for issue #%d", sub_issue.issue.number) - if sub_issue.is_open: - continue # only closed issues are reported in release notes - - if not sub_issue.contains_change_increment(): - continue # skip sub-issues without change increment - - logger.debug("Sub-issue #%s contains change increment", sub_issue.issue.number) - sub_issue_block = "- " + sub_issue.to_chapter_row() + logger.debug("Rendering sub-issue row for issue #%s", sub_issue.issue.number) + if self.is_open: + if sub_issue.is_open: + continue # only closed issues are reported in release notes + if not sub_issue.contains_change_increment(): + continue # skip sub-issues without change increment + # Closed parent: render all sub-issues regardless of state or change increment + + logger.debug("Rendering sub-issue #%s", sub_issue.issue.number) + open_icon_prefix = "" + if self.is_closed and sub_issue.is_open: + # Highlight open children under a closed parent to signal incomplete work + open_icon_prefix = ActionInputs.get_open_hierarchy_sub_issue_icon() + " " + sub_issue_block = "- " + open_icon_prefix + sub_issue.to_chapter_row() ind_child_block = "\n".join( f"{sub_indent}{line}" if line else "" for line in sub_issue_block.splitlines() ) diff --git a/release_notes_generator/utils/constants.py b/release_notes_generator/utils/constants.py index df4e5970..5f11a912 100644 --- a/release_notes_generator/utils/constants.py +++ b/release_notes_generator/utils/constants.py @@ -27,6 +27,7 @@ SUPER_CHAPTERS = "super-chapters" DUPLICITY_SCOPE = "duplicity-scope" DUPLICITY_ICON = "duplicity-icon" +OPEN_HIERARCHY_SUB_ISSUE_ICON = "open-hierarchy-sub-issue-icon" PUBLISHED_AT = "published-at" SKIP_RELEASE_NOTES_LABELS = "skip-release-notes-labels" VERBOSE = "verbose" diff --git a/release_notes_generator/utils/gh_action.py b/release_notes_generator/utils/gh_action.py index c7f0faa1..94aaf68f 100644 --- a/release_notes_generator/utils/gh_action.py +++ b/release_notes_generator/utils/gh_action.py @@ -20,10 +20,9 @@ import os import sys -from typing import Optional -def get_action_input(name: str, default: Optional[str] = None) -> Optional[str]: +def get_action_input(name: str, default: str = "") -> str: """ Retrieve the value of a specified input parameter from environment variables. @@ -32,9 +31,6 @@ def get_action_input(name: str, default: Optional[str] = None) -> Optional[str]: @return: The value of the specified input parameter, or an empty string if the environment """ - if default is None: - return os.getenv(f'INPUT_{name.replace("-", "_").upper()}') - return os.getenv(f'INPUT_{name.replace("-", "_").upper()}', default=default) diff --git a/tests/unit/conftest.py b/tests/unit/conftest.py index 58ad2d19..b1d81d28 100644 --- a/tests/unit/conftest.py +++ b/tests/unit/conftest.py @@ -17,6 +17,7 @@ from datetime import datetime, timedelta import pytest +from pytest_mock import MockerFixture from github import Github, IssueType, NamedUser from github.Commit import Commit @@ -1263,3 +1264,108 @@ def mock_logging_setup(mocker): """Fixture to mock the basic logging setup using pytest-mock.""" mock_log_config = mocker.patch("logging.basicConfig") yield mock_log_config + + +# Helpers for hierarchy issue record tests +_HIERARCHY_ROW_FMT = "_{title}_ {number}" +_ISSUE_ROW_FMT_MINIMAL = "{number} _{title}_" + + +def make_minimal_issue(mocker: MockerFixture, state: str, number: int) -> Issue: + """Return a minimal Issue mock.""" + issue = mocker.Mock(spec=Issue) + issue.state = state + issue.number = number + issue.title = "Minimal test issue" + issue.body = None + issue.type = None + issue.user = None + issue.assignees = [] + issue.get_labels.return_value = [] + return issue + + +def make_minimal_pr(mocker: MockerFixture, number: int) -> PullRequest: + """Return a minimal PullRequest mock.""" + pr = mocker.Mock(spec=PullRequest) + pr.number = number + pr.body = None + pr.html_url = f"https://github.com/org/repo/pull/{number}" + pr.user = None + pr.assignees = [] + pr.get_labels.return_value = [] + return pr + + +def make_closed_sub_issue_record_with_pr(mocker: MockerFixture, number: int) -> SubIssueRecord: + """Return a closed SubIssueRecord with one PR (no commits).""" + sub_record = SubIssueRecord(make_minimal_issue(mocker, IssueRecord.ISSUE_STATE_CLOSED, number)) + sub_record.register_pull_request(make_minimal_pr(mocker, number=number + 1000)) + return sub_record + + +def make_open_sub_issue_record_no_pr(mocker: MockerFixture, number: int) -> SubIssueRecord: + """Return an open SubIssueRecord with no PRs.""" + return SubIssueRecord(make_minimal_issue(mocker, IssueRecord.ISSUE_STATE_OPEN, number)) + + +def make_closed_sub_issue_record_no_pr(mocker: MockerFixture, number: int) -> SubIssueRecord: + """Return a closed SubIssueRecord with no PRs (no change increment).""" + return SubIssueRecord(make_minimal_issue(mocker, IssueRecord.ISSUE_STATE_CLOSED, number)) + + +def make_open_sub_hierarchy_record_no_pr(mocker: MockerFixture, number: int) -> HierarchyIssueRecord: + """Return an open HierarchyIssueRecord with no PRs.""" + return HierarchyIssueRecord(make_minimal_issue(mocker, IssueRecord.ISSUE_STATE_OPEN, number)) + + +def make_open_sub_hierarchy_record_with_pr(mocker: MockerFixture, number: int) -> HierarchyIssueRecord: + """Return an open HierarchyIssueRecord with one PR (no commits).""" + rec = HierarchyIssueRecord(make_minimal_issue(mocker, IssueRecord.ISSUE_STATE_OPEN, number)) + rec.register_pull_request(make_minimal_pr(mocker, number=number + 1000)) + return rec + + +def make_closed_sub_hierarchy_record_with_pr(mocker: MockerFixture, number: int) -> HierarchyIssueRecord: + """Return a closed HierarchyIssueRecord with one PR (no commits).""" + rec = HierarchyIssueRecord(make_minimal_issue(mocker, IssueRecord.ISSUE_STATE_CLOSED, number)) + rec.register_pull_request(make_minimal_pr(mocker, number=number + 1000)) + return rec + + +def make_closed_sub_hierarchy_record_no_pr(mocker: MockerFixture, number: int) -> HierarchyIssueRecord: + """Return a closed HierarchyIssueRecord with no PRs (no change increment).""" + return HierarchyIssueRecord(make_minimal_issue(mocker, IssueRecord.ISSUE_STATE_CLOSED, number)) + + +@pytest.fixture +def patch_hierarchy_action_inputs(mocker): + """Patch ActionInputs with minimal row-format values for HierarchyIssueRecord tests.""" + mocker.patch( + "release_notes_generator.model.record.hierarchy_issue_record.ActionInputs.get_row_format_hierarchy_issue", + return_value=_HIERARCHY_ROW_FMT, + ) + mocker.patch( + "release_notes_generator.model.record.hierarchy_issue_record.ActionInputs.get_duplicity_icon", + return_value="🔔", + ) + mocker.patch( + "release_notes_generator.model.record.issue_record.ActionInputs.get_row_format_issue", + return_value=_ISSUE_ROW_FMT_MINIMAL, + ) + mocker.patch( + "release_notes_generator.model.record.issue_record.ActionInputs.get_duplicity_icon", + return_value="🔔", + ) + mocker.patch( + "release_notes_generator.model.record.record.ActionInputs.get_release_notes_title", + return_value="Release Notes:", + ) + mocker.patch( + "release_notes_generator.model.record.record.ActionInputs.is_coderabbit_support_active", + return_value=False, + ) + mocker.patch( + "release_notes_generator.model.record.hierarchy_issue_record.ActionInputs.get_open_hierarchy_sub_issue_icon", + return_value="🟡", + ) diff --git a/tests/unit/release_notes_generator/model/test_hierarchy_issue_record.py b/tests/unit/release_notes_generator/model/test_hierarchy_issue_record.py index d9f6ba6e..a8fb515f 100644 --- a/tests/unit/release_notes_generator/model/test_hierarchy_issue_record.py +++ b/tests/unit/release_notes_generator/model/test_hierarchy_issue_record.py @@ -19,6 +19,172 @@ from release_notes_generator.model.record.hierarchy_issue_record import HierarchyIssueRecord from release_notes_generator.model.record.issue_record import IssueRecord +from tests.unit.conftest import ( + make_closed_sub_hierarchy_record_no_pr, + make_closed_sub_hierarchy_record_with_pr, + make_closed_sub_issue_record_no_pr, + make_closed_sub_issue_record_with_pr, + make_minimal_issue, + make_minimal_pr, + make_open_sub_hierarchy_record_no_pr, + make_open_sub_hierarchy_record_with_pr, + make_open_sub_issue_record_no_pr, +) + + +def test_to_chapter_row_closed_parent_renders_closed_and_open_sub_issues(mocker, patch_hierarchy_action_inputs): + """ + Closed parent with one closed sub-issue (has PR) and one open sub-issue (no change + increment) → both appear in to_chapter_row() output. + """ + parent = make_minimal_issue(mocker, IssueRecord.ISSUE_STATE_CLOSED, number=300) + record = HierarchyIssueRecord(parent) + + record.sub_issues["450"] = make_closed_sub_issue_record_with_pr(mocker, number=450) + record.sub_issues["400"] = make_open_sub_issue_record_no_pr(mocker, number=400) + + row = record.to_chapter_row() + + assert "#450" in row, f"Closed sub-issue #450 must appear; got:\n{row}" + assert "#400" in row, f"Open sub-issue #400 must appear when parent is closed; got:\n{row}" + + +def test_to_chapter_row_closed_parent_highlights_open_sub_issue(mocker, patch_hierarchy_action_inputs): + """Open sub-issue under a closed parent is prefixed with the open-hierarchy-sub-issue-icon.""" + parent = make_minimal_issue(mocker, IssueRecord.ISSUE_STATE_CLOSED, number=300) + record = HierarchyIssueRecord(parent) + + record.sub_issues["400"] = make_open_sub_issue_record_no_pr(mocker, number=400) + record.sub_issues["450"] = make_closed_sub_issue_record_with_pr(mocker, number=450) + + row = record.to_chapter_row() + + lines = row.splitlines() + open_line = next((l for l in lines if "#400" in l), None) + closed_line = next((l for l in lines if "#450" in l), None) + assert open_line is not None, f"Open sub-issue #400 must be present; got:\n{row}" + assert "🟡" in open_line, f"Open sub-issue line must contain icon; got: {open_line!r}" + assert closed_line is not None, f"Closed sub-issue #450 must be present; got:\n{row}" + assert "🟡" not in closed_line, f"Closed sub-issue line must NOT contain icon; got: {closed_line!r}" + + +def test_to_chapter_row_closed_parent_highlights_open_sub_hierarchy_issue(mocker, patch_hierarchy_action_inputs): + """Open sub-hierarchy-issue under a closed parent is prefixed with the open-hierarchy-sub-issue-icon.""" + parent = make_minimal_issue(mocker, IssueRecord.ISSUE_STATE_CLOSED, number=300) + record = HierarchyIssueRecord(parent) + + record.sub_hierarchy_issues["360"] = make_open_sub_hierarchy_record_with_pr(mocker, number=360) + record.sub_hierarchy_issues["350"] = make_closed_sub_hierarchy_record_with_pr(mocker, number=350) + + row = record.to_chapter_row() + + lines = row.splitlines() + open_line = next((l for l in lines if "#360" in l), None) + closed_line = next((l for l in lines if "#350" in l), None) + assert open_line is not None, f"Open sub-hierarchy #360 must be present; got:\n{row}" + assert "🟡" in open_line, f"Open sub-hierarchy line must contain icon; got: {open_line!r}" + assert closed_line is not None, f"Closed sub-hierarchy #350 must be present; got:\n{row}" + assert "🟡" not in closed_line, f"Closed sub-hierarchy line must NOT contain icon; got: {closed_line!r}" + + +def test_to_chapter_row_open_parent_only_renders_closed_sub_issues_with_change_increment(mocker, patch_hierarchy_action_inputs): + """ + Open parent with one closed sub-issue (has PR) and one open sub-issue (no PR) → + only the closed sub-issue appears; open sub-issue is suppressed. + """ + parent = make_minimal_issue(mocker, IssueRecord.ISSUE_STATE_OPEN, number=300) + record = HierarchyIssueRecord(parent) + + record.sub_issues["450"] = make_closed_sub_issue_record_with_pr(mocker, number=450) + record.sub_issues["400"] = make_open_sub_issue_record_no_pr(mocker, number=400) + + row = record.to_chapter_row() + + assert "#450" in row, f"Closed sub-issue #450 must appear; got:\n{row}" + assert "#400" not in row, f"Open sub-issue #400 must NOT appear when parent is open; got:\n{row}" + + +def test_to_chapter_row_open_parent_suppresses_closed_sub_issue_from_previous_release(mocker, patch_hierarchy_action_inputs): + """ + Open parent with one closed sub-issue that has no change increment (closed or delivered + in a previous release) → the sub-issue does not appear in the current release notes. + """ + parent = make_minimal_issue(mocker, IssueRecord.ISSUE_STATE_OPEN, number=300) + record = HierarchyIssueRecord(parent) + + record.sub_issues["450"] = make_closed_sub_issue_record_no_pr(mocker, number=450) + + row = record.to_chapter_row() + + assert "#450" not in row, f"Sub-issue from previous release must NOT appear; got:\n{row}" + + +def test_to_chapter_row_open_parent_suppresses_closed_sub_hierarchy_issue_from_previous_release(mocker, patch_hierarchy_action_inputs): + """ + Open parent with one closed sub-hierarchy issue that has no change increment (closed or + delivered in a previous release) → the sub-hierarchy issue does not appear. + """ + parent = make_minimal_issue(mocker, IssueRecord.ISSUE_STATE_OPEN, number=300) + record = HierarchyIssueRecord(parent) + + record.sub_hierarchy_issues["350"] = make_closed_sub_hierarchy_record_no_pr(mocker, number=350) + + row = record.to_chapter_row() + + assert "#350" not in row, f"Sub-hierarchy issue from previous release must NOT appear; got:\n{row}" + + +def test_to_chapter_row_closed_parent_renders_closed_and_open_sub_hierarchy_issues(mocker, patch_hierarchy_action_inputs): + """ + Closed parent with one closed sub-hierarchy issue (has PR) and one open sub-hierarchy + issue (no change increment) → both appear in to_chapter_row() output. + """ + parent = make_minimal_issue(mocker, IssueRecord.ISSUE_STATE_CLOSED, number=300) + record = HierarchyIssueRecord(parent) + + record.sub_hierarchy_issues["350"] = make_closed_sub_hierarchy_record_with_pr(mocker, number=350) + record.sub_hierarchy_issues["360"] = make_open_sub_hierarchy_record_no_pr(mocker, number=360) + + row = record.to_chapter_row() + + assert "#350" in row, f"Closed sub-hierarchy issue #350 must appear; got:\n{row}" + assert "#360" in row, f"Open sub-hierarchy issue #360 must appear when parent is closed; got:\n{row}" + + +def test_to_chapter_row_open_parent_only_renders_sub_hierarchy_issues_with_change_increment(mocker, patch_hierarchy_action_inputs): + """ + Open parent with one closed sub-hierarchy issue (has PR) and one open sub-hierarchy + issue (no PR, no change increment) → only the closed sub-hierarchy issue appears. + Unlike leaf sub-issues, sub-hierarchy issues are filtered by change increment only; + an open sub-hierarchy issue that aggregates PRs from its own sub-issues would still appear. + """ + parent = make_minimal_issue(mocker, IssueRecord.ISSUE_STATE_OPEN, number=300) + record = HierarchyIssueRecord(parent) + + record.sub_hierarchy_issues["350"] = make_closed_sub_hierarchy_record_with_pr(mocker, number=350) + record.sub_hierarchy_issues["360"] = make_open_sub_hierarchy_record_no_pr(mocker, number=360) + + row = record.to_chapter_row() + + assert "#350" in row, f"Closed sub-hierarchy issue #350 must appear; got:\n{row}" + assert "#360" not in row, f"Sub-hierarchy issue #360 with no change increment must NOT appear; got:\n{row}" + + +def test_to_chapter_row_open_parent_renders_open_sub_hierarchy_issue_with_change_increment(mocker, patch_hierarchy_action_inputs): + """ + Open parent with one open sub-hierarchy issue that has a PR (change increment present) + → the sub-hierarchy issue appears. Sub-hierarchy children are filtered by change + increment only; open state alone is not a reason to suppress them. + """ + parent = make_minimal_issue(mocker, IssueRecord.ISSUE_STATE_OPEN, number=300) + record = HierarchyIssueRecord(parent) + + record.sub_hierarchy_issues["360"] = make_open_sub_hierarchy_record_with_pr(mocker, number=360) + + row = record.to_chapter_row() + + assert "#360" in row, f"Open sub-hierarchy issue #360 with a PR must appear under open parent; got:\n{row}" + def test_progress_leaf_node_returns_empty_string(make_hierarchy_issue): @@ -205,24 +371,14 @@ def test_progress_per_level_independence(make_hierarchy_issue, make_sub_issue): assert child_c.progress == "", f"child_c: {child_c.progress!r}" -def _make_mock_pull(mocker, number: int): - """Create a minimal mock PullRequest with the given number.""" - from github.PullRequest import PullRequest as GHPullRequest - - pull = mocker.Mock(spec=GHPullRequest) - pull.number = number - pull.get_labels.return_value = [] - return pull - - def test_contains_change_increment_false_when_all_sub_issues_open(mocker, make_hierarchy_issue, make_sub_issue): """Bug regression: open hierarchy with only open sub-issues (with PRs) must not appear in release notes.""" parent = HierarchyIssueRecord(make_hierarchy_issue(10, IssueRecord.ISSUE_STATE_OPEN)) sub1 = make_sub_issue(11, IssueRecord.ISSUE_STATE_OPEN) - sub1.register_pull_request(_make_mock_pull(mocker, 111)) + sub1.register_pull_request(make_minimal_pr(mocker, 111)) sub2 = make_sub_issue(12, IssueRecord.ISSUE_STATE_OPEN) - sub2.register_pull_request(_make_mock_pull(mocker, 112)) + sub2.register_pull_request(make_minimal_pr(mocker, 112)) parent.sub_issues.update({"org/repo#11": sub1, "org/repo#12": sub2}) assert parent.contains_change_increment() is False @@ -233,9 +389,9 @@ def test_contains_change_increment_true_when_one_closed_sub_issue_has_pr(mocker, parent = HierarchyIssueRecord(make_hierarchy_issue(20, IssueRecord.ISSUE_STATE_OPEN)) open_sub = make_sub_issue(21, IssueRecord.ISSUE_STATE_OPEN) - open_sub.register_pull_request(_make_mock_pull(mocker, 211)) + open_sub.register_pull_request(make_minimal_pr(mocker, 211)) closed_sub = make_sub_issue(22, IssueRecord.ISSUE_STATE_CLOSED) - closed_sub.register_pull_request(_make_mock_pull(mocker, 212)) + closed_sub.register_pull_request(make_minimal_pr(mocker, 212)) parent.sub_issues.update({"org/repo#21": open_sub, "org/repo#22": closed_sub}) assert parent.contains_change_increment() is True @@ -251,7 +407,7 @@ def test_contains_change_increment_false_leaf_no_prs(make_hierarchy_issue): def test_contains_change_increment_true_leaf_with_direct_pr(mocker, make_hierarchy_issue): """A hierarchy issue with a direct PR on itself (not from sub-issues) returns True.""" record = HierarchyIssueRecord(make_hierarchy_issue(40, IssueRecord.ISSUE_STATE_OPEN)) - record.register_pull_request(_make_mock_pull(mocker, 401)) + record.register_pull_request(make_minimal_pr(mocker, 401)) assert record.contains_change_increment() is True @@ -274,7 +430,7 @@ def test_contains_change_increment_false_nested_open_only(mocker, make_hierarchy child = HierarchyIssueRecord(make_hierarchy_issue(61, IssueRecord.ISSUE_STATE_OPEN)) open_leaf = make_sub_issue(62, IssueRecord.ISSUE_STATE_OPEN) - open_leaf.register_pull_request(_make_mock_pull(mocker, 621)) + open_leaf.register_pull_request(make_minimal_pr(mocker, 621)) child.sub_issues.update({"org/repo#62": open_leaf}) root.sub_hierarchy_issues.update({"org/repo#61": child}) @@ -291,7 +447,7 @@ def test_contains_change_increment_true_nested_with_closed_leaf(mocker, make_hie child = HierarchyIssueRecord(make_hierarchy_issue(71, IssueRecord.ISSUE_STATE_OPEN)) closed_leaf = make_sub_issue(72, IssueRecord.ISSUE_STATE_CLOSED) - closed_leaf.register_pull_request(_make_mock_pull(mocker, 721)) + closed_leaf.register_pull_request(make_minimal_pr(mocker, 721)) child.sub_issues.update({"org/repo#72": closed_leaf}) root.sub_hierarchy_issues.update({"org/repo#71": child}) diff --git a/tests/unit/release_notes_generator/utils/test_gh_action.py b/tests/unit/release_notes_generator/utils/test_gh_action.py index 3a9bec1e..12831d43 100644 --- a/tests/unit/release_notes_generator/utils/test_gh_action.py +++ b/tests/unit/release_notes_generator/utils/test_gh_action.py @@ -22,18 +22,18 @@ def test_get_input_with_hyphen(mocker): mock_getenv = mocker.patch("os.getenv", return_value="test_value") - result = get_action_input("test-input", default=None) + result = get_action_input("test-input") - mock_getenv.assert_called_with("INPUT_TEST_INPUT") + mock_getenv.assert_called_with("INPUT_TEST_INPUT", default="") assert "test_value" == result def test_get_input_without_hyphen(mocker): mock_getenv = mocker.patch("os.getenv", return_value="another_test_value") - result = get_action_input("anotherinput", default=None) + result = get_action_input("anotherinput") - mock_getenv.assert_called_with("INPUT_ANOTHERINPUT") + mock_getenv.assert_called_with("INPUT_ANOTHERINPUT", default="") assert "another_test_value" == result From 30754f39b200852f5fdc47e310a6ea09e6348f10 Mon Sep 17 00:00:00 2001 From: miroslavpojer Date: Thu, 2 Apr 2026 10:17:56 +0200 Subject: [PATCH 07/20] feat: add super-chapters support for grouping chapters by label --- docs/features/custom_chapters.md | 49 +++++++ .../chapters/custom_chapters.py | 136 +++++++++++++++++- tests/unit/conftest.py | 1 + 3 files changed, 185 insertions(+), 1 deletion(-) diff --git a/docs/features/custom_chapters.md b/docs/features/custom_chapters.md index 489fd146..886563a7 100644 --- a/docs/features/custom_chapters.md +++ b/docs/features/custom_chapters.md @@ -264,6 +264,55 @@ When super chapters are configured the output uses `##` headings for super chapt - Non-dict entries are skipped with a warning. - Empty labels after normalization cause the entry to be skipped with a warning. +## Super Chapters + +**Super chapters** group regular chapters under higher-level headings based on a separate label. This is useful in monorepo or multi-module projects where the same chapter structure (Features, Bugfixes, …) should appear once per component. + +### Configuration + +Define super chapters via the `super-chapters` input — a YAML array with `title` and `label`/`labels`: + +```yaml +with: + super-chapters: | + - title: "Atum Server" + label: "atum-server" + - title: "Atum Agent" + labels: "atum-agent, atum-agent-spark" + chapters: | + - {"title": "Enhancements", "label": "enhancement"} + - {"title": "Bugfixes", "label": "bug"} +``` + +### Rendering + +When super chapters are configured the output uses `##` headings for super chapters and `###` for regular chapters nested inside: + +```markdown +## Atum Server +### Enhancements +- #10 _Streaming API_ developed by @alice in #12 + +### Bugfixes +- #15 _Fix timeout_ developed by @bob in #16 + +## Atum Agent +### Enhancements +- #20 _Checkpointing_ developed by @carol in #22 +``` + +### Behavior +- A record is placed under a super chapter if it carries at least one label matching the super chapter's labels. +- A record can appear in **multiple** super chapters if its labels match more than one. +- Within each super chapter, records are routed to regular chapters by the normal label matching rules. +- Empty super chapters (no matching records) respect the `print-empty-chapters` setting. +- When no super chapters are configured, output is flat (unchanged from previous behavior). + +### Validation +- Entries missing `title` or `label`/`labels` are skipped with a warning. +- Non-dict entries are skipped with a warning. +- Empty labels after normalization cause the entry to be skipped with a warning. + ## Related Features - [Duplicity Handling](./duplicity_handling.md) – governs multi-chapter visibility. - [Release Notes Extraction](./release_notes_extraction.md) – provides the change increment lines. diff --git a/release_notes_generator/chapters/custom_chapters.py b/release_notes_generator/chapters/custom_chapters.py index e9baa4bb..bb2f70c9 100644 --- a/release_notes_generator/chapters/custom_chapters.py +++ b/release_notes_generator/chapters/custom_chapters.py @@ -22,6 +22,8 @@ import logging from dataclasses import dataclass, field from typing import Any +from dataclasses import dataclass, field +from typing import Any, Iterable from release_notes_generator.action_inputs import ActionInputs from release_notes_generator.chapters.base_chapters import BaseChapters @@ -43,11 +45,54 @@ class SuperChapter: labels: list[str] = field(default_factory=list) +def _normalize_labels(raw: Any) -> list[str]: # helper for multi-label + """Normalize a raw labels definition into an ordered, de-duplicated list. + + Parameters: + - str: may contain newlines and/or commas + - list[str]: already a sequence of labels (will still be trimmed & deduped) + + Returns: + Ordered list preserving first occurrence order; excludes empty tokens. + Invalid (non str/list) returns empty list to be handled by caller. + """ + if isinstance(raw, list): + working: Iterable[Any] = raw + elif isinstance(raw, str): + # newline first then comma per spec + parts: list[str] = [] + for line in raw.splitlines(): + for token in line.split(","): + parts.append(token) + working = parts + else: + return [] + + cleaned: list[str] = [] + seen: set[str] = set() + for item in working: + if not isinstance(item, str): # skip non-string items silently + continue + token = item.strip() + if not token: # skip empty after trimming + continue + if token in seen: # de-duplicate preserving first occurrence + continue + seen.add(token) + cleaned.append(token) + return cleaned + + class CustomChapters(BaseChapters): """ A class used to represent the custom chapters in the release notes. """ + def __init__(self, sort_ascending: bool = True, print_empty_chapters: bool = True): + super().__init__(sort_ascending, print_empty_chapters) + self._super_chapters: list[SuperChapter] = [] + self._record_labels: dict[str, list[str]] = {} + def __init__(self, sort_ascending: bool = True, print_empty_chapters: bool = True): super().__init__(sort_ascending, print_empty_chapters) self._super_chapters: list[SuperChapter] = [] @@ -113,7 +158,10 @@ def populate(self, records: dict[str, Record]) -> None: continue record_labels = getattr(record, "labels", []) - self._record_labels[record_id] = list(record_labels) + + # Track labels for super-chapter grouping at render time + if record_labels: + self._record_labels[record_id] = list(record_labels) # Conditional Custom Chapter gate: intercept open hierarchy parents before label routing. # Note: precedes the record_labels early-exit so label-less HierarchyIssueRecord @@ -159,6 +207,11 @@ def to_string(self) -> str: A record may appear in multiple super chapters. Chapters with no matching records under a given super chapter are governed by ``print_empty_chapters``. + When super chapters are configured, records are grouped under super-chapter + headings (``## Title``) with regular chapters nested inside (``### Title``). + A record may appear in multiple super chapters. Chapters with no matching + records under a given super chapter are governed by ``print_empty_chapters``. + Returns: str: The chapters as a string with hidden chapters filtered out. """ @@ -167,6 +220,13 @@ def to_string(self) -> str: return self._to_string_flat() + def _to_string_flat(self) -> str: + """Render chapters without super-chapter grouping (original behaviour).""" + if self._super_chapters: + return self._to_string_with_super_chapters() + + return self._to_string_flat() + def _to_string_flat(self) -> str: """Render chapters without super-chapter grouping (original behaviour).""" result = "" @@ -256,6 +316,42 @@ def _to_string_with_super_chapters(self) -> str: return result.strip() + def _to_string_with_super_chapters(self) -> str: + """Render chapters grouped under super-chapter headings.""" + result = "" + for sc in self._super_chapters: + # Collect record IDs whose labels match this super chapter + matching_ids: set[str] = set() + for rid, labels in self._record_labels.items(): + if any(lbl in sc.labels for lbl in labels): + matching_ids.add(rid) + + sc_block = "" + for chapter in self._sorted_chapters(): + if chapter.hidden: + continue + + # Filter chapter rows to only those matching the super chapter + filtered_rows = { + rid: row for rid, row in chapter.rows.items() if str(rid) in matching_ids or rid in matching_ids + } + if not filtered_rows and not self.print_empty_chapters: + continue + + sorted_items = sorted(filtered_rows.items(), key=lambda item: item[0], reverse=not self.sort_ascending) + if sorted_items: + ch_str = f"### {chapter.title}\n" + "".join(f"- {value}\n" for _, value in sorted_items) + sc_block += ch_str.strip() + "\n\n" + elif self.print_empty_chapters: + sc_block += f"### {chapter.title}\n{chapter.empty_message}\n\n" + + if sc_block.strip(): + result += f"## {sc.title}\n{sc_block}" + elif self.print_empty_chapters: + result += f"## {sc.title}\nNo entries detected.\n\n" + + return result.strip() + def from_yaml_array(self, chapters: list[dict[str, Any]]) -> "CustomChapters": # type: ignore[override] """ Populate the custom chapters from a list of dict objects. @@ -325,6 +421,9 @@ def from_yaml_array(self, chapters: list[dict[str, Any]]) -> "CustomChapters": # Parse super-chapter definitions from action inputs self._super_chapters = self._parse_super_chapters(ActionInputs.get_super_chapters()) + # Parse super-chapter definitions from action inputs + self._super_chapters = self._parse_super_chapters(ActionInputs.get_super_chapters()) + return self @staticmethod @@ -339,6 +438,41 @@ def _parse_super_chapters(validated: list[dict[str, Any]]) -> list[SuperChapter] """ return [SuperChapter(title=e["title"], labels=e["labels"]) for e in validated] + @staticmethod + def _parse_super_chapters(raw_super_chapters: list[dict[str, Any]]) -> list[SuperChapter]: + """Parse super-chapter YAML definitions into SuperChapter instances. + + Parameters: + raw_super_chapters: Parsed YAML list of dicts with 'title' and 'label'/'labels'. + + Returns: + List of validated SuperChapter instances; invalid entries are skipped with a warning. + """ + result: list[SuperChapter] = [] + for entry in raw_super_chapters: + if not isinstance(entry, dict): + logger.warning("Skipping super-chapter definition with invalid type %s: %s", type(entry), entry) + continue + if "title" not in entry: + logger.warning("Skipping super-chapter without title key: %s", entry) + continue + title = entry["title"] + + raw_labels = entry.get("labels", entry.get("label")) + if raw_labels is None: + logger.warning("Super-chapter '%s' has no 'label' or 'labels' key; skipping", title) + continue + if isinstance(raw_labels, str): + raw_labels = [raw_labels] + normalized = _normalize_labels(raw_labels) + if not normalized: + logger.warning("Super-chapter '%s' labels definition empty after normalization; skipping", title) + continue + + result.append(SuperChapter(title=title, labels=normalized)) + logger.debug("Registered super-chapter '%s' with labels: %s", title, normalized) + return result + @staticmethod def _parse_bool_flag(title: str, raw: Any, key: str) -> bool: """Parse and validate a boolean flag from a chapter config entry. diff --git a/tests/unit/conftest.py b/tests/unit/conftest.py index b1d81d28..1ac9d868 100644 --- a/tests/unit/conftest.py +++ b/tests/unit/conftest.py @@ -39,6 +39,7 @@ from pytest_mock import MockerFixture +from release_notes_generator.action_inputs import ActionInputs from release_notes_generator.chapters.custom_chapters import CustomChapters from release_notes_generator.model.record.sub_issue_record import SubIssueRecord from release_notes_generator.utils.enums import DuplicityScopeEnum From 203a8cfdaff397ee987474991468f589356b3a01 Mon Sep 17 00:00:00 2001 From: miroslavpojer Date: Mon, 6 Apr 2026 09:34:00 +0200 Subject: [PATCH 08/20] Fixed after merge issues. --- release_notes_generator/action_inputs.py | 3 +- .../chapters/custom_chapters.py | 131 ------------------ .../data/utils/bulk_sub_issue_collector.py | 9 +- 3 files changed, 10 insertions(+), 133 deletions(-) diff --git a/release_notes_generator/action_inputs.py b/release_notes_generator/action_inputs.py index c0a7e316..fd5aaf82 100644 --- a/release_notes_generator/action_inputs.py +++ b/release_notes_generator/action_inputs.py @@ -596,7 +596,8 @@ def validate_inputs() -> None: logger.debug("CodeRabbit summary ignore groups: %s", coderabbit_summary_ignore_groups) logger.debug("Hidden service chapters: %s", ActionInputs.get_hidden_service_chapters()) logger.debug("Service chapter order: %s", ActionInputs.get_service_chapter_order()) - logger.debug("Super chapters: %s", ActionInputs.get_super_chapters()) + super_chapters = ActionInputs.get_super_chapters() + logger.debug("Super chapters: %s", super_chapters) @staticmethod def _detect_row_format_invalid_keywords(row_format: str, row_type: str = "Issue", clean: bool = False) -> str: diff --git a/release_notes_generator/chapters/custom_chapters.py b/release_notes_generator/chapters/custom_chapters.py index bb2f70c9..27dc6785 100644 --- a/release_notes_generator/chapters/custom_chapters.py +++ b/release_notes_generator/chapters/custom_chapters.py @@ -22,8 +22,6 @@ import logging from dataclasses import dataclass, field from typing import Any -from dataclasses import dataclass, field -from typing import Any, Iterable from release_notes_generator.action_inputs import ActionInputs from release_notes_generator.chapters.base_chapters import BaseChapters @@ -45,54 +43,11 @@ class SuperChapter: labels: list[str] = field(default_factory=list) -def _normalize_labels(raw: Any) -> list[str]: # helper for multi-label - """Normalize a raw labels definition into an ordered, de-duplicated list. - - Parameters: - - str: may contain newlines and/or commas - - list[str]: already a sequence of labels (will still be trimmed & deduped) - - Returns: - Ordered list preserving first occurrence order; excludes empty tokens. - Invalid (non str/list) returns empty list to be handled by caller. - """ - if isinstance(raw, list): - working: Iterable[Any] = raw - elif isinstance(raw, str): - # newline first then comma per spec - parts: list[str] = [] - for line in raw.splitlines(): - for token in line.split(","): - parts.append(token) - working = parts - else: - return [] - - cleaned: list[str] = [] - seen: set[str] = set() - for item in working: - if not isinstance(item, str): # skip non-string items silently - continue - token = item.strip() - if not token: # skip empty after trimming - continue - if token in seen: # de-duplicate preserving first occurrence - continue - seen.add(token) - cleaned.append(token) - return cleaned - - class CustomChapters(BaseChapters): """ A class used to represent the custom chapters in the release notes. """ - def __init__(self, sort_ascending: bool = True, print_empty_chapters: bool = True): - super().__init__(sort_ascending, print_empty_chapters) - self._super_chapters: list[SuperChapter] = [] - self._record_labels: dict[str, list[str]] = {} - def __init__(self, sort_ascending: bool = True, print_empty_chapters: bool = True): super().__init__(sort_ascending, print_empty_chapters) self._super_chapters: list[SuperChapter] = [] @@ -207,11 +162,6 @@ def to_string(self) -> str: A record may appear in multiple super chapters. Chapters with no matching records under a given super chapter are governed by ``print_empty_chapters``. - When super chapters are configured, records are grouped under super-chapter - headings (``## Title``) with regular chapters nested inside (``### Title``). - A record may appear in multiple super chapters. Chapters with no matching - records under a given super chapter are governed by ``print_empty_chapters``. - Returns: str: The chapters as a string with hidden chapters filtered out. """ @@ -220,13 +170,6 @@ def to_string(self) -> str: return self._to_string_flat() - def _to_string_flat(self) -> str: - """Render chapters without super-chapter grouping (original behaviour).""" - if self._super_chapters: - return self._to_string_with_super_chapters() - - return self._to_string_flat() - def _to_string_flat(self) -> str: """Render chapters without super-chapter grouping (original behaviour).""" result = "" @@ -316,42 +259,6 @@ def _to_string_with_super_chapters(self) -> str: return result.strip() - def _to_string_with_super_chapters(self) -> str: - """Render chapters grouped under super-chapter headings.""" - result = "" - for sc in self._super_chapters: - # Collect record IDs whose labels match this super chapter - matching_ids: set[str] = set() - for rid, labels in self._record_labels.items(): - if any(lbl in sc.labels for lbl in labels): - matching_ids.add(rid) - - sc_block = "" - for chapter in self._sorted_chapters(): - if chapter.hidden: - continue - - # Filter chapter rows to only those matching the super chapter - filtered_rows = { - rid: row for rid, row in chapter.rows.items() if str(rid) in matching_ids or rid in matching_ids - } - if not filtered_rows and not self.print_empty_chapters: - continue - - sorted_items = sorted(filtered_rows.items(), key=lambda item: item[0], reverse=not self.sort_ascending) - if sorted_items: - ch_str = f"### {chapter.title}\n" + "".join(f"- {value}\n" for _, value in sorted_items) - sc_block += ch_str.strip() + "\n\n" - elif self.print_empty_chapters: - sc_block += f"### {chapter.title}\n{chapter.empty_message}\n\n" - - if sc_block.strip(): - result += f"## {sc.title}\n{sc_block}" - elif self.print_empty_chapters: - result += f"## {sc.title}\nNo entries detected.\n\n" - - return result.strip() - def from_yaml_array(self, chapters: list[dict[str, Any]]) -> "CustomChapters": # type: ignore[override] """ Populate the custom chapters from a list of dict objects. @@ -421,9 +328,6 @@ def from_yaml_array(self, chapters: list[dict[str, Any]]) -> "CustomChapters": # Parse super-chapter definitions from action inputs self._super_chapters = self._parse_super_chapters(ActionInputs.get_super_chapters()) - # Parse super-chapter definitions from action inputs - self._super_chapters = self._parse_super_chapters(ActionInputs.get_super_chapters()) - return self @staticmethod @@ -438,41 +342,6 @@ def _parse_super_chapters(validated: list[dict[str, Any]]) -> list[SuperChapter] """ return [SuperChapter(title=e["title"], labels=e["labels"]) for e in validated] - @staticmethod - def _parse_super_chapters(raw_super_chapters: list[dict[str, Any]]) -> list[SuperChapter]: - """Parse super-chapter YAML definitions into SuperChapter instances. - - Parameters: - raw_super_chapters: Parsed YAML list of dicts with 'title' and 'label'/'labels'. - - Returns: - List of validated SuperChapter instances; invalid entries are skipped with a warning. - """ - result: list[SuperChapter] = [] - for entry in raw_super_chapters: - if not isinstance(entry, dict): - logger.warning("Skipping super-chapter definition with invalid type %s: %s", type(entry), entry) - continue - if "title" not in entry: - logger.warning("Skipping super-chapter without title key: %s", entry) - continue - title = entry["title"] - - raw_labels = entry.get("labels", entry.get("label")) - if raw_labels is None: - logger.warning("Super-chapter '%s' has no 'label' or 'labels' key; skipping", title) - continue - if isinstance(raw_labels, str): - raw_labels = [raw_labels] - normalized = _normalize_labels(raw_labels) - if not normalized: - logger.warning("Super-chapter '%s' labels definition empty after normalization; skipping", title) - continue - - result.append(SuperChapter(title=title, labels=normalized)) - logger.debug("Registered super-chapter '%s' with labels: %s", title, normalized) - return result - @staticmethod def _parse_bool_flag(title: str, raw: Any, key: str) -> bool: """Parse and validate a boolean flag from a chapter config entry. diff --git a/release_notes_generator/data/utils/bulk_sub_issue_collector.py b/release_notes_generator/data/utils/bulk_sub_issue_collector.py index e0819114..e51885e7 100644 --- a/release_notes_generator/data/utils/bulk_sub_issue_collector.py +++ b/release_notes_generator/data/utils/bulk_sub_issue_collector.py @@ -230,10 +230,17 @@ def _post_graphql(self, payload: dict) -> dict: logger.debug("Posted graphql query") return data except Exception as e: # pylint: disable=broad-exception-caught - logger.exception("GraphQL query failed") last_exc = e if attempt == self._cfg.max_retries: + logger.exception("GraphQL query failed after %d attempts", self._cfg.max_retries) raise + logger.warning( + "GraphQL query failed (attempt %d/%d): %s; retrying in %.1fs", + attempt, + self._cfg.max_retries, + e, + self._cfg.base_backoff * attempt, + ) time.sleep(self._cfg.base_backoff * attempt) if last_exc: raise last_exc From b93b0db5335dba71e17b83a30f53e5a1802191c7 Mon Sep 17 00:00:00 2001 From: miroslavpojer Date: Mon, 6 Apr 2026 16:10:18 +0200 Subject: [PATCH 09/20] Implemented partial split by super chapter label. --- .github/copilot-instructions.md | 5 + docs/features/custom_chapters.md | 82 ++-- docs/features/issue_hierarchy_support.md | 1 + .../chapters/custom_chapters.py | 75 ++- .../model/record/hierarchy_issue_record.py | 75 ++- .../chapters/test_custom_chapters.py | 454 +++++++++++++++++- .../model/test_hierarchy_issue_record.py | 414 ++++++++++++++++ 7 files changed, 1049 insertions(+), 57 deletions(-) diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index 34d4b526..69ca61f5 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -84,6 +84,11 @@ Testing - Must not access private members (names starting with `_`) of the class under test directly in tests. - Must place shared test helper functions and factory fixtures in the nearest `conftest.py` and reuse them across tests. - Must annotate pytest fixture parameters with `MockerFixture` (from `pytest_mock`) and return types with `Callable[..., T]` (from `collections.abc`) when the fixture returns a factory function. +- Prefer TDD workflow: + 1. 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. + 2. Be ready to add, remove, or rename tests based on user feedback before proceeding. + 3. Once confirmed, write all failing tests first (red), then implement until all pass (green). + 4. Cover all distinct combinations; each test must state its scenario in the docstring. Tooling - Must format with Black (pyproject.toml). diff --git a/docs/features/custom_chapters.md b/docs/features/custom_chapters.md index 886563a7..a6827563 100644 --- a/docs/features/custom_chapters.md +++ b/docs/features/custom_chapters.md @@ -256,57 +256,69 @@ When super chapters are configured the output uses `##` headings for super chapt - A record is placed under a super-chapter if it carries at least one label matching the super-chapter's labels. - A record can appear in **multiple** super-chapters if its labels match more than one. - Within each super-chapter, records are routed to regular chapters by the normal label-matching rules. -- Empty super chapters (no matching records) respect the `print-empty-chapters` setting. +- Empty super chapters (no matching records) respect the `print-empty-chapters` setting: + - `print-empty-chapters: true` → header is printed with `No entries detected.` + - `print-empty-chapters: false` → header is omitted entirely +- `## Uncategorized` is only emitted when there are actually unmatched records; `print-empty-chapters` has no effect on it. - When no super chapters are configured, output is flat (unchanged from previous behavior). -### Validation -- Entries missing `title` or `label`/`labels` are skipped with a warning. -- Non-dict entries are skipped with a warning. -- Empty labels after normalization cause the entry to be skipped with a warning. +### Hierarchy Split (with `hierarchy: true`) -## Super Chapters +With `hierarchy: true`, super-chapter matching uses each hierarchy record's **full aggregated label set** — own labels plus all descendant labels recursively at every depth. So an Epic matches a super chapter even when the relevant label lives only on a grandchild Task (e.g. Epic → Feature → Task). -**Super chapters** group regular chapters under higher-level headings based on a separate label. This is useful in monorepo or multi-module projects where the same chapter structure (Features, Bugfixes, …) should appear once per component. +The record is then split by which descendants belong to which super chapter: -### Configuration +| Descendants | Output | +|---|---| +| All match one super chapter | Record appears in that super chapter only | +| None match any super chapter | Record appears in `## Uncategorized` only | +| Some match, some don't | Record appears in the matching super chapter **and** in `## Uncategorized`, each showing only its own subset | +| Match multiple super chapters | Record appears in each matching super chapter with its relevant subset | +| Match multiple SCs + some unmatched | Record appears in each matching super chapter and in `## Uncategorized` | -Define super chapters via the `super-chapters` input — a YAML array with `title` and `label`/`labels`: +#### Example + +Epic #1 has Task #2 (`scope:frontend`) and Task #3 (`scope:backend`): ```yaml -with: - super-chapters: | - - title: "Atum Server" - label: "atum-server" - - title: "Atum Agent" - labels: "atum-agent, atum-agent-spark" - chapters: | - - {"title": "Enhancements", "label": "enhancement"} - - {"title": "Bugfixes", "label": "bug"} +super-chapters: | + - title: "Frontend" + label: "scope:frontend" + - title: "Backend" + label: "scope:backend" +chapters: | + - {"title": "New Features", "labels": "feature"} ``` -### Rendering +```markdown +## Frontend +### New Features +- Epic: _Add user authentication_ #1 + - #2 _Build login form_ + +## Backend +### New Features +- Epic: _Add user authentication_ #1 + - #3 _Add JWT endpoint_ +``` -When super chapters are configured the output uses `##` headings for super chapters and `###` for regular chapters nested inside: +If Epic #1 also had Task #4 with no super-chapter label, it would additionally appear in `## Uncategorized` with only Task #4. -```markdown -## Atum Server -### Enhancements -- #10 _Streaming API_ developed by @alice in #12 +#### 3-level depth -### Bugfixes -- #15 _Fix timeout_ developed by @bob in #16 +The same split works when the matching label is on a grandchild. Epic #1 → Feature #2 → Task #3 (`scope:security`): -## Atum Agent -### Enhancements -- #20 _Checkpointing_ developed by @carol in #22 +```markdown +## Security +### New Features +- Epic: _Add authentication_ #1 + - Feature: _Auth flow_ #2 + - #3 _Add JWT endpoint_ ``` -### Behavior -- A record is placed under a super chapter if it carries at least one label matching the super chapter's labels. -- A record can appear in **multiple** super chapters if its labels match more than one. -- Within each super chapter, records are routed to regular chapters by the normal label matching rules. -- Empty super chapters (no matching records) respect the `print-empty-chapters` setting. -- When no super chapters are configured, output is flat (unchanged from previous behavior). +Feature #2 has no `scope:security` label of its own, but its aggregated set includes it via Task #3, so it is routed to the Security super chapter. + +Children within each rendered node are sorted **ascending by issue number**. ### Validation - Entries missing `title` or `label`/`labels` are skipped with a warning. diff --git a/docs/features/issue_hierarchy_support.md b/docs/features/issue_hierarchy_support.md index 9cf6efb3..484e4d02 100644 --- a/docs/features/issue_hierarchy_support.md +++ b/docs/features/issue_hierarchy_support.md @@ -75,6 +75,7 @@ Resulting output (hierarchy issues only — sub-issue rows use `row-format-issue - [Custom Row Formats](./custom_row_formats.md) – controls hierarchy line rendering. - [Service Chapters](./service_chapters.md) – flags missing change increments if hierarchy parents lack qualifying sub-issues. - [Duplicity Handling](./duplicity_handling.md) – duplicate hierarchy items can be icon-prefixed if allowed. +- [Super Chapters](./custom_chapters.md#super-chapters) – hierarchy records split across super chapters when descendants carry different super-chapter labels. ← [Back to Feature Tutorials](../../README.md#feature-tutorials) diff --git a/release_notes_generator/chapters/custom_chapters.py b/release_notes_generator/chapters/custom_chapters.py index 27dc6785..9a1f1f5b 100644 --- a/release_notes_generator/chapters/custom_chapters.py +++ b/release_notes_generator/chapters/custom_chapters.py @@ -52,6 +52,7 @@ def __init__(self, sort_ascending: bool = True, print_empty_chapters: bool = Tru super().__init__(sort_ascending, print_empty_chapters) self._super_chapters: list[SuperChapter] = [] self._record_labels: dict[str, list[str]] = {} + self._records: dict[str, Record] = {} def _find_catch_open_hierarchy_chapter(self) -> Chapter | None: """Return the first chapter with catch_open_hierarchy enabled, or None.""" @@ -66,6 +67,7 @@ def _add_record_to_chapter(self, record_id: str, record: Record, chapter: Chapte if not chapter.hidden: record.add_to_chapter_presence(chapter.title) chapter.add_row(record_id, record.to_chapter_row(not chapter.hidden)) + self._records[record_id] = record if record_id not in self.populated_record_numbers_list: self.populated_record_numbers_list.append(record_id) if chapter.hidden and ActionInputs.get_verbose(): @@ -114,8 +116,14 @@ def populate(self, records: dict[str, Record]) -> None: record_labels = getattr(record, "labels", []) - # Track labels for super-chapter grouping at render time - if record_labels: + # Track labels for super-chapter grouping at render time. + # HierarchyIssueRecords use aggregated labels (own + descendants) so the + # hierarchy parent matches super chapters via sub-issue labels. + if isinstance(record, HierarchyIssueRecord): + all_labels = record.get_labels() + if all_labels: + self._record_labels[record_id] = list(all_labels) + elif record_labels: self._record_labels[record_id] = list(record_labels) # Conditional Custom Chapter gate: intercept open hierarchy parents before label routing. @@ -198,12 +206,40 @@ def _collect_super_chapter_ids(self, sc: SuperChapter) -> set[str]: matching.add(rid) return matching - def _render_chapter_for_ids(self, chapter: Chapter, matching_ids: set[str]) -> str: - """Render a single chapter filtered to matching_ids, delegating to Chapter.to_string.""" + def _render_chapter_for_ids( + self, + chapter: Chapter, + matching_ids: set[str], + super_labels: list[str] | None = None, + exclude_labels: list[str] | None = None, + ) -> str: + """Render a single chapter filtered to matching_ids, delegating to Chapter.to_string. + + Parameters: + chapter: The chapter to render. + matching_ids: Record IDs to include. + super_labels: When set, HierarchyIssueRecords are re-rendered with sub-issues + filtered to those whose labels intersect *super_labels*. + exclude_labels: When set, HierarchyIssueRecords are re-rendered with sub-issues + whose labels intersect *exclude_labels* removed (for Uncategorized fallback). TODO - make this test simpler and descriptive + """ original_rows = chapter.rows - chapter.rows = { - rid: row for rid, row in original_rows.items() if str(rid) in matching_ids or rid in matching_ids - } + filtered_rows: dict[int | str, str] = {} + for rid, row in original_rows.items(): + if str(rid) not in matching_ids and rid not in matching_ids: + continue + record = self._records.get(str(rid)) or self._records.get(rid) # type: ignore[arg-type] + if super_labels and isinstance(record, HierarchyIssueRecord): + if not record.has_matching_labels(super_labels): + continue + filtered_rows[rid] = record.to_chapter_row(add_into_chapters=False, label_filter=super_labels) + elif exclude_labels and isinstance(record, HierarchyIssueRecord): + filtered_rows[rid] = record.to_chapter_row( + add_into_chapters=False, exclude_labels=exclude_labels + ) + else: + filtered_rows[rid] = row + chapter.rows = filtered_rows try: return chapter.to_string(sort_ascending=self.sort_ascending, print_empty_chapters=self.print_empty_chapters) finally: @@ -216,6 +252,8 @@ def _to_string_with_super_chapters(self) -> str: for sc in self._super_chapters: all_super_labels.update(sc.labels) + all_super_labels_list = list(all_super_labels) + claimed_ids: set[str] = set() for rid, labels in self._record_labels.items(): if any(lbl in all_super_labels for lbl in labels): @@ -229,7 +267,7 @@ def _to_string_with_super_chapters(self) -> str: for chapter in self._sorted_chapters(): if chapter.hidden: continue - ch_str = self._render_chapter_for_ids(chapter, matching_ids) + ch_str = self._render_chapter_for_ids(chapter, matching_ids, super_labels=sc.labels) if ch_str: sc_block += ch_str + "\n\n" @@ -238,20 +276,33 @@ def _to_string_with_super_chapters(self) -> str: elif self.print_empty_chapters: result += f"## {sc.title}\nNo entries detected.\n\n" - # Fallback: records not claimed by any super chapter + # Fallback: records not claimed by any super chapter. + # Hierarchy records that ARE claimed but have unmatched descendants also appear + # in Uncategorized with exclude_labels to render only the non-SC portion. unclaimed_ids: set[str] = set() + partial_hierarchy_ids: set[str] = set() for chapter in self._sorted_chapters(): for row_id in chapter.rows: row_id_str = str(row_id) if row_id_str not in claimed_ids: unclaimed_ids.add(row_id_str) - - if unclaimed_ids: + elif row_id_str in claimed_ids: + record = self._records.get(row_id_str) + if ( + isinstance(record, HierarchyIssueRecord) + and record.has_unmatched_descendants(all_super_labels_list) + ): + partial_hierarchy_ids.add(row_id_str) + + all_uncat_ids = unclaimed_ids | partial_hierarchy_ids + if all_uncat_ids: uc_block = "" for chapter in self._sorted_chapters(): if chapter.hidden: continue - ch_str = self._render_chapter_for_ids(chapter, unclaimed_ids) + ch_str = self._render_chapter_for_ids( + chapter, all_uncat_ids, exclude_labels=all_super_labels_list + ) if ch_str: uc_block += ch_str + "\n\n" if uc_block.strip(): diff --git a/release_notes_generator/model/record/hierarchy_issue_record.py b/release_notes_generator/model/record/hierarchy_issue_record.py index 9b429584..99294b9a 100644 --- a/release_notes_generator/model/record/hierarchy_issue_record.py +++ b/release_notes_generator/model/record/hierarchy_issue_record.py @@ -167,15 +167,62 @@ def get_labels(self) -> list[str]: labels.update(sub_issue.labels) for sub_hierarchy_issue in self._sub_hierarchy_issues.values(): - labels.update(sub_hierarchy_issue.labels) + labels.update(sub_hierarchy_issue.get_labels()) for pull in self._pull_requests.values(): labels.update(label.name for label in pull.get_labels()) return list(labels) + def has_matching_labels(self, label_filter: list[str]) -> bool: + """Check if this hierarchy issue or any descendant has labels matching the filter. + + Parameters: + label_filter: Labels to check against. + + Returns: + True if this record or any descendant has at least one matching label. + """ + if any(lbl in label_filter for lbl in self.labels): + return True + for sub_issue in self._sub_issues.values(): + if any(lbl in label_filter for lbl in sub_issue.labels): + return True + for sub_hierarchy_issue in self._sub_hierarchy_issues.values(): + if sub_hierarchy_issue.has_matching_labels(label_filter): + return True + return False + + def has_unmatched_descendants(self, all_super_labels: list[str]) -> bool: + """Check if any descendant does NOT match any label in the combined super-chapter label set. + + Parameters: + all_super_labels: Union of all super-chapter labels. + + Returns: + True if at least one descendant has no label intersecting *all_super_labels*. + """ + for sub_issue in self._sub_issues.values(): + if not any(lbl in all_super_labels for lbl in sub_issue.labels): + return True + for sub_hierarchy_issue in self._sub_hierarchy_issues.values(): + if sub_hierarchy_issue.has_unmatched_descendants(all_super_labels): + return True + # A leaf HierarchyIssueRecord (no children of its own) is unmatched if + # its own labels don't intersect the SC set. Intermediate nodes are + # pure containers — their own organisational labels are irrelevant. + is_leaf = not sub_hierarchy_issue.sub_issues and not sub_hierarchy_issue.sub_hierarchy_issues + if is_leaf and not any(lbl in all_super_labels for lbl in sub_hierarchy_issue.labels): + return True + return False + # methods - override ancestor methods - def to_chapter_row(self, add_into_chapters: bool = True) -> str: + def to_chapter_row( + self, + add_into_chapters: bool = True, + label_filter: list[str] | None = None, + exclude_labels: list[str] | None = None, + ) -> str: logger.debug("Rendering hierarchy issue row for issue #%s", self.issue.number) row_prefix = f"{ActionInputs.get_duplicity_icon()} " if self.chapter_presence_count() > 1 else "" format_values: dict[str, Any] = {} @@ -215,15 +262,25 @@ def to_chapter_row(self, add_into_chapters: bool = True) -> str: row = f"{row}\n{rls_block}" # add sub-hierarchy issues - for sub_hierarchy_issue in self._sub_hierarchy_issues.values(): + for sub_hierarchy_issue in sorted(self._sub_hierarchy_issues.values(), key=lambda r: r.issue.number): logger.debug("Rendering sub-hierarchy issue row for #%s", sub_hierarchy_issue.issue.number) + if label_filter and not sub_hierarchy_issue.has_matching_labels(label_filter): + continue + if ( + exclude_labels + and not sub_hierarchy_issue.has_unmatched_descendants(exclude_labels) + and sub_hierarchy_issue.has_matching_labels(exclude_labels) + ): + continue if self.is_open: if not sub_hierarchy_issue.contains_change_increment(): continue # Closed parent: render all sub-hierarchy issues regardless of state or change increment logger.debug("Rendering sub-hierarchy issue #%s", sub_hierarchy_issue.issue.number) if self.is_closed and sub_hierarchy_issue.is_open: - sub_row = sub_hierarchy_issue.to_chapter_row() + sub_row = sub_hierarchy_issue.to_chapter_row( + label_filter=label_filter, exclude_labels=exclude_labels + ) # Highlight open children under a closed parent to signal incomplete work icon = ActionInputs.get_open_hierarchy_sub_issue_icon() header_line, newline, remaining_lines = sub_row.partition("\n") @@ -231,14 +288,20 @@ def to_chapter_row(self, add_into_chapters: bool = True) -> str: indent = header_line[: len(header_line) - len(header_text)] sub_row = f"{indent}{icon} {header_text}{newline}{remaining_lines}" else: - sub_row = sub_hierarchy_issue.to_chapter_row() + sub_row = sub_hierarchy_issue.to_chapter_row( + label_filter=label_filter, exclude_labels=exclude_labels + ) row = f"{row}\n{sub_row}" # add sub-issues if len(self._sub_issues) > 0: sub_indent = " " * (self._level + 1) - for sub_issue in self._sub_issues.values(): + for sub_issue in sorted(self._sub_issues.values(), key=lambda r: r.issue.number): logger.debug("Rendering sub-issue row for issue #%s", sub_issue.issue.number) + if label_filter and not any(lbl in label_filter for lbl in sub_issue.labels): + continue + if exclude_labels and any(lbl in exclude_labels for lbl in sub_issue.labels): + continue if self.is_open: if sub_issue.is_open: continue # only closed issues are reported in release notes diff --git a/tests/unit/release_notes_generator/chapters/test_custom_chapters.py b/tests/unit/release_notes_generator/chapters/test_custom_chapters.py index de3b1cf2..ccc6df82 100644 --- a/tests/unit/release_notes_generator/chapters/test_custom_chapters.py +++ b/tests/unit/release_notes_generator/chapters/test_custom_chapters.py @@ -826,9 +826,6 @@ def test_sorted_chapters_hidden_with_order(): assert sorted_chs[2].title == "Visible2" -# ------- Tests for catch-open-hierarchy (Feature 1) ------- - - @pytest.fixture def hierarchy_record_stub(): """Create a minimal HierarchyIssueRecord-like stub for catch-open-hierarchy tests.""" @@ -875,7 +872,7 @@ def assignees(self): def developers(self): return [] - def to_chapter_row(self, add_into_chapters=True): + def to_chapter_row(self, add_into_chapters=True, label_filter=None, exclude_labels=None): return f"{self._rid} row" def contains_change_increment(self): @@ -884,6 +881,9 @@ def contains_change_increment(self): def get_labels(self): return self.labels + def has_unmatched_descendants(self, all_super_labels): + return False + def get_rls_notes(self, _line_marks=None): return "" @@ -1392,3 +1392,449 @@ def test_super_chapters_coh_record_visible_in_fallback(mocker, hierarchy_record_ # Assert - COH record appears in Uncategorized since it has no super-chapter label assert "## Uncategorized" in output assert "org/repo#H1 row" in output + + +@pytest.fixture +def hierarchy_with_sub_issues_stub(): + """Create a HierarchyIssueRecord-like stub that renders sub-issues filtered by label_filter.""" + + class SubIssueStub: + def __init__(self, rid, labels): + self.rid = rid + self._labels = labels + + @property + def labels(self): + return self._labels + + class HierarchyWithSubsStub(HierarchyIssueRecord): + """Stub with sub-issue-aware to_chapter_row that respects label_filter.""" + + # noinspection PyMissingConstructor + def __init__(self, rid, labels, sub_issue_stubs, state="closed"): + Record.__init__(self, labels=labels, skip=False) + self._rid = rid + self._state = state + self._contains = True + self._level = 0 + self._sub_issues = {} + self._sub_hierarchy_issues = {} + self._issue = None + self._pull_requests = {} + self._commits = {} + self._issue_type = None + self._sub_stubs: list[SubIssueStub] = sub_issue_stubs + + @property + def record_id(self): + return self._rid + + @property + def is_closed(self): + return self._state == "closed" + + @property + def is_open(self): + return self._state == "open" + + @property + def author(self): + return "author" + + @property + def assignees(self): + return [] + + @property + def developers(self): + return [] + + def to_chapter_row(self, add_into_chapters=True, label_filter=None, exclude_labels=None): + row = f"{self._rid} row" + for stub in self._sub_stubs: + if label_filter and not any(lbl in label_filter for lbl in stub.labels): + continue + if exclude_labels and any(lbl in exclude_labels for lbl in stub.labels): + continue + row += f"\n {stub.rid} sub-row" + return row + + def contains_change_increment(self): + return self._contains + + def get_labels(self): + all_labels = list(self._labels or []) + for stub in self._sub_stubs: + all_labels.extend(stub.labels) + return all_labels + + def has_matching_labels(self, label_filter): + if any(lbl in label_filter for lbl in (self._labels or [])): + return True + for stub in self._sub_stubs: + if any(lbl in label_filter for lbl in stub.labels): + return True + return False + + def has_unmatched_descendants(self, all_super_labels): + if not self._sub_stubs: + return False + for stub in self._sub_stubs: + if not any(lbl in all_super_labels for lbl in stub.labels): + return True + return False + + def get_rls_notes(self, _line_marks=None): + return "" + + def _make(rid, labels, sub_issue_stubs, state="closed"): + return HierarchyWithSubsStub(rid, labels, sub_issue_stubs, state) + + _make.SubIssueStub = SubIssueStub + return _make + + +def test_super_chapters_hierarchy_sub_issues_filtered_by_service(mocker, hierarchy_with_sub_issues_stub): + """Epic with frontend + backend tasks renders only relevant sub-issues per super chapter.""" + # Arrange + SI = hierarchy_with_sub_issues_stub.SubIssueStub + epic = hierarchy_with_sub_issues_stub( + rid="org/repo#1", + labels=["epic", "enhancement"], + sub_issue_stubs=[ + SI("org/repo#2", ["enhancement", "frontend"]), + SI("org/repo#3", ["enhancement", "backend"]), + ], + ) + cc = make_super_chapters_cc( + mocker, + [{"title": "New Features", "label": "enhancement"}], + [ + {"title": "Frontend", "labels": ["frontend"]}, + {"title": "Backend", "labels": ["backend"]}, + ], + ) + cc.populate({"org/repo#1": epic}) + # Act + output = cc.to_string() + # Assert structure + assert "## Frontend" in output + assert "## Backend" in output + fe_section = output.split("## Backend")[0] + be_section = output.split("## Backend")[1] + # Frontend section has only sub-issue #2 + assert "org/repo#2 sub-row" in fe_section + assert "org/repo#3 sub-row" not in fe_section + # Backend section has only sub-issue #3 + assert "org/repo#3 sub-row" in be_section + assert "org/repo#2 sub-row" not in be_section + + +def test_super_chapters_hierarchy_no_matching_descendants_hides_epic(mocker, hierarchy_with_sub_issues_stub): + """Epic is hidden from a super chapter if none of its descendants match.""" + SI = hierarchy_with_sub_issues_stub.SubIssueStub + epic = hierarchy_with_sub_issues_stub( + rid="org/repo#1", + labels=["epic", "enhancement"], + sub_issue_stubs=[SI("org/repo#2", ["enhancement", "frontend"])], + ) + cc = make_super_chapters_cc( + mocker, + [{"title": "New Features", "label": "enhancement"}], + [ + {"title": "Frontend", "labels": ["frontend"]}, + {"title": "Backend", "labels": ["backend"]}, + ], + print_empty=False, + ) + cc.populate({"org/repo#1": epic}) + # Act + output = cc.to_string() + # Assert + assert "## Frontend" in output + assert "## Backend" not in output, "Backend should be skipped when no matching descendants" + + +def test_sc_combo_c1_all_descendants_match_one_sc(mocker, hierarchy_with_sub_issues_stub): + """When every descendant matches a single super chapter, epic appears only there, not in Uncategorized.""" + SI = hierarchy_with_sub_issues_stub.SubIssueStub + epic = hierarchy_with_sub_issues_stub( + rid="org/repo#1", + labels=["epic", "enhancement"], + sub_issue_stubs=[ + SI("org/repo#2", ["enhancement", "scope:security"]), + SI("org/repo#3", ["enhancement", "scope:security"]), + ], + ) + cc = make_super_chapters_cc( + mocker, + [{"title": "New Features", "label": "enhancement"}], + [{"title": "Security", "labels": ["scope:security"]}], + ) + cc.populate({"org/repo#1": epic}) + output = cc.to_string() + + assert "## Security" in output + assert "org/repo#1 row" in output + assert "org/repo#2 sub-row" in output + assert "org/repo#3 sub-row" in output + assert "Uncategorized" not in output + + +def test_sc_combo_c2_no_descendants_match_any_sc(mocker, hierarchy_with_sub_issues_stub): + """When no descendant matches any super chapter, epic appears only in Uncategorized.""" + SI = hierarchy_with_sub_issues_stub.SubIssueStub + epic = hierarchy_with_sub_issues_stub( + rid="org/repo#1", + labels=["epic", "enhancement"], + sub_issue_stubs=[ + SI("org/repo#2", ["enhancement", "frontend"]), + SI("org/repo#3", ["enhancement", "backend"]), + ], + ) + cc = make_super_chapters_cc( + mocker, + [{"title": "New Features", "label": "enhancement"}], + [{"title": "Security", "labels": ["scope:security"]}], + ) + cc.populate({"org/repo#1": epic}) + output = cc.to_string() + + assert "## Security" in output # header present (empty or not) + security_section = output.split("## Security")[1].split("\n## ")[0] if "## Security" in output else "" + assert "org/repo#1 row" not in security_section + assert "Uncategorized" in output + uncat_section = output.split("Uncategorized")[1] + assert "org/repo#1 row" in uncat_section + assert "org/repo#2 sub-row" in uncat_section + assert "org/repo#3 sub-row" in uncat_section + + +def test_sc_combo_c3_split_descendants_between_sc_and_uncategorized(mocker, hierarchy_with_sub_issues_stub): + """Epic with security-labeled and non-security-labeled descendants appears in both SC and Uncategorized. + + This is the user-reported scenario: + Epic (labels: epic, scope:security) + Task A (labels: enhancement, scope:security) → Security SC + Task B (labels: enhancement) → Uncategorized + """ + SI = hierarchy_with_sub_issues_stub.SubIssueStub + epic = hierarchy_with_sub_issues_stub( + rid="org/repo#1", + labels=["epic", "enhancement"], + sub_issue_stubs=[ + SI("org/repo#2", ["enhancement", "scope:security"]), + SI("org/repo#3", ["enhancement"]), + ], + ) + cc = make_super_chapters_cc( + mocker, + [{"title": "New Features", "label": "enhancement"}], + [{"title": "Security", "labels": ["scope:security"]}], + ) + cc.populate({"org/repo#1": epic}) + output = cc.to_string() + + # Security SC contains epic with only the security sub-issue + assert "## Security" in output + security_section = output.split("## Security")[1].split("\n## ")[0] + assert "org/repo#1 row" in security_section + assert "org/repo#2 sub-row" in security_section + assert "org/repo#3 sub-row" not in security_section + + # Uncategorized contains the same epic with only the non-security sub-issue + assert "Uncategorized" in output + uncat_section = output.split("Uncategorized")[1] + assert "org/repo#1 row" in uncat_section + assert "org/repo#3 sub-row" in uncat_section + assert "org/repo#2 sub-row" not in uncat_section + + +def test_sc_combo_c3b_three_level_hierarchy_split(mocker, hierarchy_with_sub_issues_stub): + """Three-level scenario: Epic → Feature → Tasks; Feature has mixed SC/non-SC descendants. + + Epic (labels: epic, enhancement) + Feature (labels: security) — aggregates scope:security from Task2 + Task1 (enhancement) → Uncategorized + Task2 (enhancement, scope:security) → Security SC + + Expected: + ## Security → Epic row + Feature sub-row containing Task2 + ## Uncategorized → Epic row + Feature sub-row containing Task1 + """ + SI = hierarchy_with_sub_issues_stub.SubIssueStub + + task1_si = SI("org/repo#3", ["enhancement"]) + task2_si = SI("org/repo#4", ["enhancement", "scope:security"]) + + # Feature stub with two sub-issues + feature = hierarchy_with_sub_issues_stub( + rid="org/repo#2", + labels=["security"], + sub_issue_stubs=[task1_si, task2_si], + ) + + # Epic stub with feature as a sub-hierarchy + epic = hierarchy_with_sub_issues_stub( + rid="org/repo#1", + labels=["epic", "enhancement"], + sub_issue_stubs=[SI("org/repo#2", ["security", "scope:security", "enhancement"])], + ) + # Override to_chapter_row and helper methods so Feature renders correctly + epic._sub_stubs = [] + + original_to_row = epic.to_chapter_row.__func__ if hasattr(epic.to_chapter_row, "__func__") else None + + def epic_to_chapter_row(add_into_chapters=True, label_filter=None, exclude_labels=None): + row = f"org/repo#1 row" + sub_row = feature.to_chapter_row( + add_into_chapters=add_into_chapters, label_filter=label_filter, exclude_labels=exclude_labels + ) + if sub_row: + row += f"\n {sub_row}" + return row + + import types + epic.to_chapter_row = epic_to_chapter_row + + def epic_has_matching_labels(label_filter): + if any(lbl in label_filter for lbl in ["epic", "enhancement"]): + return True + return feature.has_matching_labels(label_filter) + + def epic_has_unmatched_descendants(all_super_labels): + return feature.has_unmatched_descendants(all_super_labels) + + def epic_get_labels(): + return ["epic", "enhancement"] + feature.get_labels() + + epic.has_matching_labels = epic_has_matching_labels + epic.has_unmatched_descendants = epic_has_unmatched_descendants + epic.get_labels = epic_get_labels + epic._labels = ["epic", "enhancement"] + feature.get_labels() + + cc = make_super_chapters_cc( + mocker, + [{"title": "New Features", "label": "enhancement"}], + [{"title": "Security", "labels": ["scope:security"]}], + ) + cc.populate({"org/repo#1": epic}) + output = cc.to_string() + + # Security SC: shows epic with feature fragment containing only task2 + assert "## Security" in output + sec_section = output.split("## Security")[1].split("\n## ")[0] + assert "org/repo#1 row" in sec_section + assert "org/repo#4 sub-row" in sec_section + assert "org/repo#3 sub-row" not in sec_section + + # Uncategorized: same epic with feature fragment containing only task1 + assert "Uncategorized" in output + uncat_section = output.split("Uncategorized")[1] + assert "org/repo#1 row" in uncat_section + assert "org/repo#3 sub-row" in uncat_section + assert "org/repo#4 sub-row" not in uncat_section + + +def test_sc_combo_c4_descendants_match_multiple_scs(mocker, hierarchy_with_sub_issues_stub): + """Epic with descendants matching different super chapters appears in each, filtered appropriately.""" + SI = hierarchy_with_sub_issues_stub.SubIssueStub + epic = hierarchy_with_sub_issues_stub( + rid="org/repo#1", + labels=["epic", "enhancement"], + sub_issue_stubs=[ + SI("org/repo#2", ["enhancement", "frontend"]), + SI("org/repo#3", ["enhancement", "backend"]), + ], + ) + cc = make_super_chapters_cc( + mocker, + [{"title": "New Features", "label": "enhancement"}], + [ + {"title": "Frontend", "labels": ["frontend"]}, + {"title": "Backend", "labels": ["backend"]}, + ], + ) + cc.populate({"org/repo#1": epic}) + output = cc.to_string() + + # Epic appears in both super chapters + assert "## Frontend" in output + assert "## Backend" in output + fe_section = output.split("## Backend")[0] + be_section = output.split("## Backend")[1] + assert "org/repo#2 sub-row" in fe_section + assert "org/repo#3 sub-row" not in fe_section + assert "org/repo#3 sub-row" in be_section + assert "org/repo#2 sub-row" not in be_section + # Every descendant is covered → no Uncategorized + assert "Uncategorized" not in output + + +def test_sc_combo_c5_multi_sc_plus_uncategorized(mocker, hierarchy_with_sub_issues_stub): + """Epic with descendants for two SCs plus an unmatched descendant appears in all three sections.""" + SI = hierarchy_with_sub_issues_stub.SubIssueStub + epic = hierarchy_with_sub_issues_stub( + rid="org/repo#1", + labels=["epic", "enhancement"], + sub_issue_stubs=[ + SI("org/repo#2", ["enhancement", "frontend"]), + SI("org/repo#3", ["enhancement", "backend"]), + SI("org/repo#4", ["enhancement"]), + ], + ) + cc = make_super_chapters_cc( + mocker, + [{"title": "New Features", "label": "enhancement"}], + [ + {"title": "Frontend", "labels": ["frontend"]}, + {"title": "Backend", "labels": ["backend"]}, + ], + ) + cc.populate({"org/repo#1": epic}) + output = cc.to_string() + + # Frontend + assert "## Frontend" in output + fe_section = output.split("## Backend")[0] + assert "org/repo#2 sub-row" in fe_section + assert "org/repo#3 sub-row" not in fe_section + assert "org/repo#4 sub-row" not in fe_section + + # Backend + assert "## Backend" in output + # Backend section is between "## Backend" and next "\n## " or end + be_section = output.split("## Backend")[1].split("\n## ")[0] + assert "org/repo#3 sub-row" in be_section + assert "org/repo#2 sub-row" not in be_section + assert "org/repo#4 sub-row" not in be_section + + # Uncategorized + assert "Uncategorized" in output + uncat_section = output.split("Uncategorized")[1] + assert "org/repo#4 sub-row" in uncat_section + assert "org/repo#2 sub-row" not in uncat_section + assert "org/repo#3 sub-row" not in uncat_section + + +def test_sc_combo_c6_plain_record_routing_unchanged(mocker, record_stub): + """Non-hierarchy records follow standard label-based super-chapter routing (no split logic).""" + cc = make_super_chapters_cc( + mocker, + [{"title": "New Features", "label": "enhancement"}], + [{"title": "Security", "labels": ["scope:security"]}], + ) + r1 = record_stub("org/repo#10", ["enhancement", "scope:security"]) + r2 = record_stub("org/repo#11", ["enhancement"]) + cc.populate({"org/repo#10": r1, "org/repo#11": r2}) + output = cc.to_string() + + # r1 goes to Security SC + assert "## Security" in output + security_section = output.split("## Security")[1].split("\n## ")[0] + assert "org/repo#10 row" in security_section + + # r2 goes to Uncategorized + assert "Uncategorized" in output + uncat_section = output.split("Uncategorized")[1] + assert "org/repo#11 row" in uncat_section diff --git a/tests/unit/release_notes_generator/model/test_hierarchy_issue_record.py b/tests/unit/release_notes_generator/model/test_hierarchy_issue_record.py index a8fb515f..993938de 100644 --- a/tests/unit/release_notes_generator/model/test_hierarchy_issue_record.py +++ b/tests/unit/release_notes_generator/model/test_hierarchy_issue_record.py @@ -19,6 +19,7 @@ from release_notes_generator.model.record.hierarchy_issue_record import HierarchyIssueRecord from release_notes_generator.model.record.issue_record import IssueRecord +from release_notes_generator.model.record.sub_issue_record import SubIssueRecord from tests.unit.conftest import ( make_closed_sub_hierarchy_record_no_pr, make_closed_sub_hierarchy_record_with_pr, @@ -452,3 +453,416 @@ def test_contains_change_increment_true_nested_with_closed_leaf(mocker, make_hie root.sub_hierarchy_issues.update({"org/repo#71": child}) assert root.contains_change_increment() is True + + +# --- has_matching_labels --- + + +def test_has_matching_labels_own_label_matches(make_hierarchy_issue): + """Parent's own labels trigger a match.""" + issue = make_hierarchy_issue(100, IssueRecord.ISSUE_STATE_OPEN) + issue.get_labels.return_value = [type("L", (), {"name": "frontend"})()] + record = HierarchyIssueRecord(issue, issue_labels=["frontend"]) + + assert record.has_matching_labels(["frontend"]) is True + + +def test_has_matching_labels_sub_issue_label_matches(mocker, make_hierarchy_issue): + """A sub-issue label satisfies the filter even when the parent has no matching labels.""" + issue = make_hierarchy_issue(100, IssueRecord.ISSUE_STATE_OPEN) + record = HierarchyIssueRecord(issue, issue_labels=["epic"]) + + sub_issue = make_minimal_issue(mocker, IssueRecord.ISSUE_STATE_CLOSED, number=200) + sub_record = SubIssueRecord(sub_issue, issue_labels=["frontend"]) + record.sub_issues["200"] = sub_record + + assert record.has_matching_labels(["frontend"]) is True + assert record.has_matching_labels(["backend"]) is False + + +def test_has_matching_labels_recursive_through_sub_hierarchy(mocker, make_hierarchy_issue): + """Label matching recurses through nested HierarchyIssueRecords.""" + root_issue = make_hierarchy_issue(100, IssueRecord.ISSUE_STATE_OPEN) + root = HierarchyIssueRecord(root_issue, issue_labels=["epic"]) + + child_issue = make_hierarchy_issue(200, IssueRecord.ISSUE_STATE_OPEN) + child = HierarchyIssueRecord(child_issue, issue_labels=["sub-epic"]) + + leaf_issue = make_minimal_issue(mocker, IssueRecord.ISSUE_STATE_CLOSED, number=300) + leaf = SubIssueRecord(leaf_issue, issue_labels=["backend"]) + child.sub_issues["300"] = leaf + + root.sub_hierarchy_issues["200"] = child + + assert root.has_matching_labels(["backend"]) is True + assert root.has_matching_labels(["frontend"]) is False + + +def test_has_matching_labels_no_match_returns_false(make_hierarchy_issue): + """Returns False when no labels match anywhere in the tree.""" + issue = make_hierarchy_issue(100, IssueRecord.ISSUE_STATE_OPEN) + record = HierarchyIssueRecord(issue, issue_labels=["epic"]) + + assert record.has_matching_labels(["frontend"]) is False + + +# --- to_chapter_row with label_filter --- + + +def test_to_chapter_row_label_filter_includes_matching_sub_issues(mocker, patch_hierarchy_action_inputs): + """Only sub-issues whose labels intersect label_filter appear in the output.""" + parent = make_minimal_issue(mocker, IssueRecord.ISSUE_STATE_CLOSED, number=1) + record = HierarchyIssueRecord(parent) + + fe_sub = make_closed_sub_issue_record_with_pr(mocker, number=2) + fe_sub._labels = ["enhancement", "frontend"] + be_sub = make_closed_sub_issue_record_with_pr(mocker, number=3) + be_sub._labels = ["enhancement", "backend"] + record.sub_issues["2"] = fe_sub + record.sub_issues["3"] = be_sub + + row = record.to_chapter_row(label_filter=["frontend"]) + + assert "#2" in row, f"Frontend sub-issue #2 should appear; got:\n{row}" + assert "#3" not in row, f"Backend sub-issue #3 should NOT appear; got:\n{row}" + + +def test_to_chapter_row_label_filter_includes_matching_sub_hierarchy(mocker, patch_hierarchy_action_inputs): + """Sub-hierarchy issues are filtered by label_filter recursively.""" + parent = make_minimal_issue(mocker, IssueRecord.ISSUE_STATE_CLOSED, number=1) + record = HierarchyIssueRecord(parent) + + fe_child = make_closed_sub_hierarchy_record_with_pr(mocker, number=10) + fe_child._labels = ["frontend"] + be_child = make_closed_sub_hierarchy_record_with_pr(mocker, number=20) + be_child._labels = ["backend"] + record.sub_hierarchy_issues["10"] = fe_child + record.sub_hierarchy_issues["20"] = be_child + + row = record.to_chapter_row(label_filter=["frontend"]) + + assert "#10" in row, f"Frontend sub-hierarchy #10 should appear; got:\n{row}" + assert "#20" not in row, f"Backend sub-hierarchy #20 should NOT appear; got:\n{row}" + + +def test_to_chapter_row_no_label_filter_renders_all(mocker, patch_hierarchy_action_inputs): + """Without label_filter, all sub-issues render (existing behaviour preserved).""" + parent = make_minimal_issue(mocker, IssueRecord.ISSUE_STATE_CLOSED, number=1) + record = HierarchyIssueRecord(parent) + + fe_sub = make_closed_sub_issue_record_with_pr(mocker, number=2) + fe_sub._labels = ["frontend"] + be_sub = make_closed_sub_issue_record_with_pr(mocker, number=3) + be_sub._labels = ["backend"] + record.sub_issues["2"] = fe_sub + record.sub_issues["3"] = be_sub + + row = record.to_chapter_row() + + assert "#2" in row + assert "#3" in row + + +def test_has_unmatched_descendants_true_when_sub_issue_has_no_matching_label(mocker, make_hierarchy_issue): + """Returns True when at least one sub-issue does not match the label set.""" + issue = make_hierarchy_issue(100, IssueRecord.ISSUE_STATE_OPEN) + record = HierarchyIssueRecord(issue, issue_labels=["epic"]) + + matched = SubIssueRecord(make_minimal_issue(mocker, IssueRecord.ISSUE_STATE_CLOSED, 200), issue_labels=["security"]) + unmatched = SubIssueRecord(make_minimal_issue(mocker, IssueRecord.ISSUE_STATE_CLOSED, 201), issue_labels=["other"]) + record.sub_issues["200"] = matched + record.sub_issues["201"] = unmatched + + assert record.has_unmatched_descendants(["security"]) is True + + +def test_has_unmatched_descendants_false_when_all_match(mocker, make_hierarchy_issue): + """Returns False when every descendant matches the label set.""" + issue = make_hierarchy_issue(100, IssueRecord.ISSUE_STATE_OPEN) + record = HierarchyIssueRecord(issue, issue_labels=["epic"]) + + s1 = SubIssueRecord(make_minimal_issue(mocker, IssueRecord.ISSUE_STATE_CLOSED, 200), issue_labels=["security"]) + s2 = SubIssueRecord(make_minimal_issue(mocker, IssueRecord.ISSUE_STATE_CLOSED, 201), issue_labels=["security", "bug"]) + record.sub_issues["200"] = s1 + record.sub_issues["201"] = s2 + + assert record.has_unmatched_descendants(["security"]) is False + + +def test_has_unmatched_descendants_recursive(mocker, make_hierarchy_issue): + """Recurses through sub-hierarchy-issues to find unmatched descendants.""" + root = HierarchyIssueRecord(make_hierarchy_issue(100, IssueRecord.ISSUE_STATE_OPEN), issue_labels=["epic"]) + child = HierarchyIssueRecord(make_hierarchy_issue(200, IssueRecord.ISSUE_STATE_OPEN), issue_labels=["sub-epic"]) + leaf = SubIssueRecord(make_minimal_issue(mocker, IssueRecord.ISSUE_STATE_CLOSED, 300), issue_labels=["other"]) + child.sub_issues["300"] = leaf + root.sub_hierarchy_issues["200"] = child + + assert root.has_unmatched_descendants(["security"]) is True + + +def test_has_unmatched_descendants_empty_tree(make_hierarchy_issue): + """A leaf hierarchy issue with no descendants returns False (nothing to be unmatched).""" + record = HierarchyIssueRecord(make_hierarchy_issue(100, IssueRecord.ISSUE_STATE_OPEN)) + + assert record.has_unmatched_descendants(["security"]) is False + + +def test_to_chapter_row_exclude_labels_hides_matching_sub_issues(mocker, patch_hierarchy_action_inputs): + """Sub-issues whose labels intersect exclude_labels are hidden from the output.""" + parent = make_minimal_issue(mocker, IssueRecord.ISSUE_STATE_CLOSED, number=1) + record = HierarchyIssueRecord(parent) + + sec_sub = make_closed_sub_issue_record_with_pr(mocker, number=2) + sec_sub._labels = ["enhancement", "scope:security"] + plain_sub = make_closed_sub_issue_record_with_pr(mocker, number=3) + plain_sub._labels = ["enhancement"] + record.sub_issues["2"] = sec_sub + record.sub_issues["3"] = plain_sub + + row = record.to_chapter_row(exclude_labels=["scope:security"]) + + assert "#3" in row, f"Non-security sub-issue should appear; got:\n{row}" + assert "#2" not in row, f"Security sub-issue should be excluded; got:\n{row}" + + +def test_to_chapter_row_exclude_labels_hides_matching_sub_hierarchy(mocker, patch_hierarchy_action_inputs): + """Sub-hierarchy issue with only SC-matching labels is hidden by exclude_labels. + + A leaf sub-hierarchy node whose own labels all match exclude_labels and which + has no unmatched descendants is excluded from the Uncategorized render. + A sibling leaf whose labels do NOT match is kept. + """ + parent = make_minimal_issue(mocker, IssueRecord.ISSUE_STATE_CLOSED, number=1) + record = HierarchyIssueRecord(parent) + + sec_child = make_closed_sub_hierarchy_record_with_pr(mocker, number=10) + sec_child._labels = ["scope:security"] + plain_child = make_closed_sub_hierarchy_record_with_pr(mocker, number=20) + plain_child._labels = ["other"] + record.sub_hierarchy_issues["10"] = sec_child + record.sub_hierarchy_issues["20"] = plain_child + + row = record.to_chapter_row(exclude_labels=["scope:security"]) + + assert "#20" in row, f"Non-security sub-hierarchy should appear; got:\n{row}" + assert "#10" not in row, f"Security sub-hierarchy should be excluded; got:\n{row}" + + +def test_to_chapter_row_exclude_labels_none_renders_all(mocker, patch_hierarchy_action_inputs): + """Without exclude_labels, all sub-issues render (backward compatible).""" + parent = make_minimal_issue(mocker, IssueRecord.ISSUE_STATE_CLOSED, number=1) + record = HierarchyIssueRecord(parent) + + s1 = make_closed_sub_issue_record_with_pr(mocker, number=2) + s1._labels = ["scope:security"] + s2 = make_closed_sub_issue_record_with_pr(mocker, number=3) + s2._labels = ["other"] + record.sub_issues["2"] = s1 + record.sub_issues["3"] = s2 + + row = record.to_chapter_row() + + assert "#2" in row + assert "#3" in row + + +def test_to_chapter_row_exclude_labels_keeps_sub_hierarchy_with_mixed_descendants(mocker, patch_hierarchy_action_inputs): + """Sub-hierarchy issue with SOME matching descendants is kept (not hidden) by exclude_labels. + + Real-data scenario: + Epic (level 0) + Feature (level 1) — aggregates scope:security from Task2 + Task1 (no scope:security) → must appear in Uncategorized + Task2 (scope:security) → must be excluded from Uncategorized + + Bug: Feature's aggregated labels include scope:security so has_matching_labels returns True, + causing the entire Feature branch to be skipped — leaving only the Epic root line. + Fix: skip Feature only when ALL its descendants are SC-covered (has_unmatched_descendants=False). + """ + epic = HierarchyIssueRecord(make_minimal_issue(mocker, IssueRecord.ISSUE_STATE_CLOSED, number=1)) + + feature = HierarchyIssueRecord(make_minimal_issue(mocker, IssueRecord.ISSUE_STATE_CLOSED, number=10)) + feature._labels = ["security"] + + task1 = make_closed_sub_issue_record_with_pr(mocker, number=20) + task1._labels = ["enhancement"] + task2 = make_closed_sub_issue_record_with_pr(mocker, number=21) + task2._labels = ["enhancement", "scope:security"] + + feature.sub_issues["20"] = task1 + feature.sub_issues["21"] = task2 + epic.sub_hierarchy_issues["10"] = feature + + row = epic.to_chapter_row(exclude_labels=["scope:security"]) + + assert "#10" in row, f"Feature should appear (has non-SC sub-issue Task1); got:\n{row}" + assert "#20" in row, f"Task1 (no SC label) should appear; got:\n{row}" + assert "#21" not in row, f"Task2 (scope:security) should be excluded; got:\n{row}" + + +def test_to_chapter_row_exclude_labels_hides_sub_hierarchy_when_all_descendants_match( + mocker, patch_hierarchy_action_inputs +): + """Sub-hierarchy issue is hidden when ALL its descendants match exclude_labels.""" + epic = HierarchyIssueRecord(make_minimal_issue(mocker, IssueRecord.ISSUE_STATE_CLOSED, number=1)) + + feature = HierarchyIssueRecord(make_minimal_issue(mocker, IssueRecord.ISSUE_STATE_CLOSED, number=10)) + feature._labels = ["security"] + + task2 = make_closed_sub_issue_record_with_pr(mocker, number=21) + task2._labels = ["enhancement", "scope:security"] + + feature.sub_issues["21"] = task2 + epic.sub_hierarchy_issues["10"] = feature + + row = epic.to_chapter_row(exclude_labels=["scope:security"]) + + assert "#10" not in row, f"Feature should be hidden (all descendants have SC label); got:\n{row}" + assert "#21" not in row, f"Task2 should be excluded; got:\n{row}" + + +def test_get_labels_aggregates_deep_sub_hierarchy_labels(mocker, make_hierarchy_issue): + """get_labels() must recursively aggregate labels from nested HierarchyIssueRecords. + + Bug: previous implementation used sub_hierarchy_issue.labels (own labels only), + so a label that exists only on a grandchild was invisible to the root Epic. + This caused the root Epic to be missing from claimed_ids and therefore never rendered + in the matching super chapter. + """ + root_issue = make_hierarchy_issue(100, IssueRecord.ISSUE_STATE_CLOSED) + root = HierarchyIssueRecord(root_issue, issue_labels=["epic"]) + + feature_issue = make_hierarchy_issue(200, IssueRecord.ISSUE_STATE_CLOSED) + feature = HierarchyIssueRecord(feature_issue, issue_labels=["feature"]) # no "scope:security" + + leaf_issue = make_minimal_issue(mocker, IssueRecord.ISSUE_STATE_CLOSED, number=300) + leaf = SubIssueRecord(leaf_issue, issue_labels=["scope:security"]) + feature.sub_issues["300"] = leaf + + root.sub_hierarchy_issues["200"] = feature + + result = root.get_labels() + + assert "scope:security" in result, ( + "Root Epic must aggregate 'scope:security' from grandchild leaf; got: " + str(result) + ) + + +def test_to_chapter_row_sub_issues_rendered_in_ascending_number_order(mocker, patch_hierarchy_action_inputs): + """Leaf sub-issues must be rendered sorted ascending by issue number regardless of insertion order.""" + parent = HierarchyIssueRecord(make_minimal_issue(mocker, IssueRecord.ISSUE_STATE_CLOSED, number=1)) + + high = make_closed_sub_issue_record_with_pr(mocker, number=4150) + low = make_closed_sub_issue_record_with_pr(mocker, number=4149) + + # Intentionally insert higher number first + parent.sub_issues["4150"] = high + parent.sub_issues["4149"] = low + + row = parent.to_chapter_row() + + pos_low = row.index("#4149") + pos_high = row.index("#4150") + assert pos_low < pos_high, f"#4149 must appear before #4150; got:\n{row}" + + +def test_to_chapter_row_sub_hierarchy_issues_rendered_in_ascending_number_order( + mocker, patch_hierarchy_action_inputs +): + """Sub-hierarchy issues must be rendered sorted ascending by issue number regardless of insertion order.""" + parent = HierarchyIssueRecord(make_minimal_issue(mocker, IssueRecord.ISSUE_STATE_CLOSED, number=1)) + + high = make_closed_sub_hierarchy_record_with_pr(mocker, number=200) + high.level = 1 + low = make_closed_sub_hierarchy_record_with_pr(mocker, number=100) + low.level = 1 + + # Intentionally insert higher number first + parent.sub_hierarchy_issues["200"] = high + parent.sub_hierarchy_issues["100"] = low + + row = parent.to_chapter_row() + + pos_low = row.index("#100") + pos_high = row.index("#200") + assert pos_low < pos_high, f"#100 must appear before #200; got:\n{row}" + + +def test_five_level_hierarchy_label_filter_and_exclude_labels(mocker, patch_hierarchy_action_inputs): + """All recursive methods work correctly at 5 levels of nesting. + + Tree: + L1: Epic #1 (labels: epic) + L2: Theme #2 (labels: theme) + L3: Feature #3 (labels: feature) + L4: Story #4 (labels: story) + L5a: Task #10 (labels: scope:security) ← SC match + L5b: Task #11 (labels: enhancement) ← no SC label + + Verified behaviours: + - get_labels() on L1 includes scope:security (aggregated from 4 levels down) + - has_matching_labels(["scope:security"]) on L1 is True + - has_unmatched_descendants(["scope:security"]) on L1 is True (Task #11 is unmatched) + - to_chapter_row(label_filter=["scope:security"]) shows only the SC path down to Task #10 + - to_chapter_row(exclude_labels=["scope:security"]) shows the non-SC path down to Task #11 + and omits Task #10 + """ + # Build bottom-up: Task leaves first + task_sc = make_closed_sub_issue_record_with_pr(mocker, number=10) + task_sc._labels = ["scope:security"] + task_plain = make_closed_sub_issue_record_with_pr(mocker, number=11) + task_plain._labels = ["enhancement"] + + story = HierarchyIssueRecord(make_minimal_issue(mocker, IssueRecord.ISSUE_STATE_CLOSED, number=4)) + story._labels = ["story"] + story._level = 3 + story.sub_issues["10"] = task_sc + story.sub_issues["11"] = task_plain + + feature = HierarchyIssueRecord(make_minimal_issue(mocker, IssueRecord.ISSUE_STATE_CLOSED, number=3)) + feature._labels = ["feature"] + feature._level = 2 + feature.sub_hierarchy_issues["4"] = story + + theme = HierarchyIssueRecord(make_minimal_issue(mocker, IssueRecord.ISSUE_STATE_CLOSED, number=2)) + theme._labels = ["theme"] + theme._level = 1 + theme.sub_hierarchy_issues["3"] = feature + + epic = HierarchyIssueRecord(make_minimal_issue(mocker, IssueRecord.ISSUE_STATE_CLOSED, number=1)) + epic._labels = ["epic"] + epic._level = 0 + epic.sub_hierarchy_issues["2"] = theme + + # --- get_labels aggregates across all 5 levels --- + all_labels = epic.get_labels() + assert "scope:security" in all_labels, f"scope:security must bubble up to L1; got {all_labels}" + assert "enhancement" in all_labels, f"enhancement must bubble up to L1; got {all_labels}" + + # --- has_matching_labels --- + assert epic.has_matching_labels(["scope:security"]) is True + assert epic.has_matching_labels(["unknown"]) is False + + # --- has_unmatched_descendants --- + # Task #11 has no scope:security → True + assert epic.has_unmatched_descendants(["scope:security"]) is True + # Both leaf labels covered → False + assert epic.has_unmatched_descendants(["scope:security", "enhancement"]) is False + + # --- to_chapter_row with label_filter: only SC path visible --- + row_sc = epic.to_chapter_row(label_filter=["scope:security"]) + assert "#1" in row_sc, f"Epic root must appear; got:\n{row_sc}" + assert "#2" in row_sc, f"Theme must appear (contains SC path); got:\n{row_sc}" + assert "#3" in row_sc, f"Feature must appear (contains SC path); got:\n{row_sc}" + assert "#4" in row_sc, f"Story must appear (contains SC path); got:\n{row_sc}" + assert "#10" in row_sc, f"SC task must appear; got:\n{row_sc}" + assert "#11" not in row_sc, f"Plain task must be excluded; got:\n{row_sc}" + + # --- to_chapter_row with exclude_labels: only non-SC path visible --- + row_plain = epic.to_chapter_row(exclude_labels=["scope:security"]) + assert "#1" in row_plain, f"Epic root must appear; got:\n{row_plain}" + assert "#2" in row_plain, f"Theme must appear (has non-SC path); got:\n{row_plain}" + assert "#3" in row_plain, f"Feature must appear (has non-SC path); got:\n{row_plain}" + assert "#4" in row_plain, f"Story must appear (has non-SC path); got:\n{row_plain}" + assert "#11" in row_plain, f"Plain task must appear; got:\n{row_plain}" + assert "#10" not in row_plain, f"SC task must be excluded; got:\n{row_plain}" From abfb0ffe252b0cc8d5fd7e6e1f258d8097edd88f Mon Sep 17 00:00:00 2001 From: miroslavpojer Date: Mon, 6 Apr 2026 16:15:47 +0200 Subject: [PATCH 10/20] fix: simplify comment in CustomChapters class for clarity --- .github/copilot-instructions.md | 11 +++++++---- release_notes_generator/chapters/custom_chapters.py | 2 +- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index 69ca61f5..96e3170b 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -85,10 +85,13 @@ Testing - Must place shared test helper functions and factory fixtures in the nearest `conftest.py` and reuse them across tests. - Must annotate pytest fixture parameters with `MockerFixture` (from `pytest_mock`) and return types with `Callable[..., T]` (from `collections.abc`) when the fixture returns a factory function. - Prefer TDD workflow: - 1. 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. - 2. Be ready to add, remove, or rename tests based on user feedback before proceeding. - 3. Once confirmed, write all failing tests first (red), then implement until all pass (green). - 4. Cover all distinct combinations; each test must state its scenario in the docstring. + 1. Create or update `SPEC.md` in the relevant package directory to describe the expected solution — list the scenarios, inputs, and expected outputs before any code is written. + 2. 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. + 3. Be ready to add, remove, or rename tests based on user feedback before proceeding. + 4. Once confirmed, write all failing tests first (red), then implement until all pass (green). + 5. Cover all distinct combinations; each test must state its scenario in the docstring. + 6. After all tests pass, update `SPEC.md` with the confirmed test case table (name + intent + input + expected output) to show what is covered. +- Must not add comments outside test methods in `test_*.py` files; use section-header comments (`# --- section ---`) only to separate logical groups of tests. Tooling - Must format with Black (pyproject.toml). diff --git a/release_notes_generator/chapters/custom_chapters.py b/release_notes_generator/chapters/custom_chapters.py index 9a1f1f5b..c1a9c625 100644 --- a/release_notes_generator/chapters/custom_chapters.py +++ b/release_notes_generator/chapters/custom_chapters.py @@ -221,7 +221,7 @@ def _render_chapter_for_ids( super_labels: When set, HierarchyIssueRecords are re-rendered with sub-issues filtered to those whose labels intersect *super_labels*. exclude_labels: When set, HierarchyIssueRecords are re-rendered with sub-issues - whose labels intersect *exclude_labels* removed (for Uncategorized fallback). TODO - make this test simpler and descriptive + whose labels intersect *exclude_labels* removed (for Uncategorized fallback). """ original_rows = chapter.rows filtered_rows: dict[int | str, str] = {} From cd6cadc54cfcc66e1bd6733aa430a3c14a491e53 Mon Sep 17 00:00:00 2001 From: miroslavpojer Date: Mon, 6 Apr 2026 16:30:59 +0200 Subject: [PATCH 11/20] refactor: streamline method calls in CustomChapters and HierarchyIssueRecord classes for improved readability --- release_notes_generator/chapters/custom_chapters.py | 13 ++++--------- .../model/record/hierarchy_issue_record.py | 8 ++------ 2 files changed, 6 insertions(+), 15 deletions(-) diff --git a/release_notes_generator/chapters/custom_chapters.py b/release_notes_generator/chapters/custom_chapters.py index c1a9c625..56fb363c 100644 --- a/release_notes_generator/chapters/custom_chapters.py +++ b/release_notes_generator/chapters/custom_chapters.py @@ -234,9 +234,7 @@ def _render_chapter_for_ids( continue filtered_rows[rid] = record.to_chapter_row(add_into_chapters=False, label_filter=super_labels) elif exclude_labels and isinstance(record, HierarchyIssueRecord): - filtered_rows[rid] = record.to_chapter_row( - add_into_chapters=False, exclude_labels=exclude_labels - ) + filtered_rows[rid] = record.to_chapter_row(add_into_chapters=False, exclude_labels=exclude_labels) else: filtered_rows[rid] = row chapter.rows = filtered_rows @@ -288,9 +286,8 @@ def _to_string_with_super_chapters(self) -> str: unclaimed_ids.add(row_id_str) elif row_id_str in claimed_ids: record = self._records.get(row_id_str) - if ( - isinstance(record, HierarchyIssueRecord) - and record.has_unmatched_descendants(all_super_labels_list) + if isinstance(record, HierarchyIssueRecord) and record.has_unmatched_descendants( + all_super_labels_list ): partial_hierarchy_ids.add(row_id_str) @@ -300,9 +297,7 @@ def _to_string_with_super_chapters(self) -> str: for chapter in self._sorted_chapters(): if chapter.hidden: continue - ch_str = self._render_chapter_for_ids( - chapter, all_uncat_ids, exclude_labels=all_super_labels_list - ) + ch_str = self._render_chapter_for_ids(chapter, all_uncat_ids, exclude_labels=all_super_labels_list) if ch_str: uc_block += ch_str + "\n\n" if uc_block.strip(): diff --git a/release_notes_generator/model/record/hierarchy_issue_record.py b/release_notes_generator/model/record/hierarchy_issue_record.py index 99294b9a..4adc00b8 100644 --- a/release_notes_generator/model/record/hierarchy_issue_record.py +++ b/release_notes_generator/model/record/hierarchy_issue_record.py @@ -278,9 +278,7 @@ def to_chapter_row( # Closed parent: render all sub-hierarchy issues regardless of state or change increment logger.debug("Rendering sub-hierarchy issue #%s", sub_hierarchy_issue.issue.number) if self.is_closed and sub_hierarchy_issue.is_open: - sub_row = sub_hierarchy_issue.to_chapter_row( - label_filter=label_filter, exclude_labels=exclude_labels - ) + sub_row = sub_hierarchy_issue.to_chapter_row(label_filter=label_filter, exclude_labels=exclude_labels) # Highlight open children under a closed parent to signal incomplete work icon = ActionInputs.get_open_hierarchy_sub_issue_icon() header_line, newline, remaining_lines = sub_row.partition("\n") @@ -288,9 +286,7 @@ def to_chapter_row( indent = header_line[: len(header_line) - len(header_text)] sub_row = f"{indent}{icon} {header_text}{newline}{remaining_lines}" else: - sub_row = sub_hierarchy_issue.to_chapter_row( - label_filter=label_filter, exclude_labels=exclude_labels - ) + sub_row = sub_hierarchy_issue.to_chapter_row(label_filter=label_filter, exclude_labels=exclude_labels) row = f"{row}\n{sub_row}" # add sub-issues From bafb0ebc0f9d9e9bb50564d19233847b3630584f Mon Sep 17 00:00:00 2001 From: miroslavpojer Date: Mon, 6 Apr 2026 16:47:33 +0200 Subject: [PATCH 12/20] Fixed review comments. --- .../model/record/hierarchy_issue_record.py | 17 +++++-- .../model/test_hierarchy_issue_record.py | 46 +++++++++++++++++++ 2 files changed, 58 insertions(+), 5 deletions(-) diff --git a/release_notes_generator/model/record/hierarchy_issue_record.py b/release_notes_generator/model/record/hierarchy_issue_record.py index 4adc00b8..7aec777f 100644 --- a/release_notes_generator/model/record/hierarchy_issue_record.py +++ b/release_notes_generator/model/record/hierarchy_issue_record.py @@ -279,12 +279,19 @@ def to_chapter_row( logger.debug("Rendering sub-hierarchy issue #%s", sub_hierarchy_issue.issue.number) if self.is_closed and sub_hierarchy_issue.is_open: sub_row = sub_hierarchy_issue.to_chapter_row(label_filter=label_filter, exclude_labels=exclude_labels) - # Highlight open children under a closed parent to signal incomplete work + # Highlight open children under a closed parent to signal incomplete work. + # Insert icon after the list marker ('- ') to preserve Markdown structure, + # and skip insertion entirely when the icon is empty. icon = ActionInputs.get_open_hierarchy_sub_issue_icon() - header_line, newline, remaining_lines = sub_row.partition("\n") - header_text = header_line.lstrip() - indent = header_line[: len(header_line) - len(header_text)] - sub_row = f"{indent}{icon} {header_text}{newline}{remaining_lines}" + if icon: + header_line, newline, remaining_lines = sub_row.partition("\n") + header_text = header_line.lstrip() + spaces = header_line[: len(header_line) - len(header_text)] + if header_text.startswith("- "): + marker, content = "- ", header_text[2:] + else: + marker, content = "", header_text + sub_row = f"{spaces}{marker}{icon} {content}{newline}{remaining_lines}" else: sub_row = sub_hierarchy_issue.to_chapter_row(label_filter=label_filter, exclude_labels=exclude_labels) row = f"{row}\n{sub_row}" diff --git a/tests/unit/release_notes_generator/model/test_hierarchy_issue_record.py b/tests/unit/release_notes_generator/model/test_hierarchy_issue_record.py index 993938de..232575f6 100644 --- a/tests/unit/release_notes_generator/model/test_hierarchy_issue_record.py +++ b/tests/unit/release_notes_generator/model/test_hierarchy_issue_record.py @@ -88,6 +88,52 @@ def test_to_chapter_row_closed_parent_highlights_open_sub_hierarchy_issue(mocker assert "🟡" not in closed_line, f"Closed sub-hierarchy line must NOT contain icon; got: {closed_line!r}" +def test_to_chapter_row_open_sub_hierarchy_icon_placed_after_list_marker(mocker, patch_hierarchy_action_inputs): + """Icon must appear after the '- ' list marker, not before it. + + Correct: ' - 🟡 _title_ #360' + Wrong: ' 🟡 - _title_ #360' + """ + parent = make_minimal_issue(mocker, IssueRecord.ISSUE_STATE_CLOSED, number=300) + record = HierarchyIssueRecord(parent) + child = make_open_sub_hierarchy_record_with_pr(mocker, number=360) + child.level = 1 + record.sub_hierarchy_issues["360"] = child + + row = record.to_chapter_row() + + open_line = next((l for l in row.splitlines() if "#360" in l), None) + assert open_line is not None + stripped = open_line.lstrip() + assert stripped.startswith("- 🟡 "), ( + f"Expected '- 🟡 ' after leading spaces; got: {open_line!r}" + ) + + +def test_to_chapter_row_empty_icon_leaves_no_stray_space(mocker, patch_hierarchy_action_inputs): + """When open-hierarchy-sub-issue-icon is '', the row indentation is unchanged (no stray space).""" + mocker.patch( + "release_notes_generator.model.record.hierarchy_issue_record.ActionInputs.get_open_hierarchy_sub_issue_icon", + return_value="", + ) + parent = make_minimal_issue(mocker, IssueRecord.ISSUE_STATE_CLOSED, number=300) + record = HierarchyIssueRecord(parent) + child = make_open_sub_hierarchy_record_with_pr(mocker, number=360) + child.level = 1 + record.sub_hierarchy_issues["360"] = child + + row = record.to_chapter_row() + + open_line = next((l for l in row.splitlines() if "#360" in l), None) + assert open_line is not None + assert open_line.startswith(" - "), ( + f"Level-1 row must start with exactly ' - ' (2 spaces); got: {open_line!r}" + ) + assert not open_line.startswith(" "), ( + f"No stray extra space in indentation with empty icon; got: {open_line!r}" + ) + + def test_to_chapter_row_open_parent_only_renders_closed_sub_issues_with_change_increment(mocker, patch_hierarchy_action_inputs): """ Open parent with one closed sub-issue (has PR) and one open sub-issue (no PR) → From 583978fe02aea9e17ff365a3cee17cc6becebfa4 Mon Sep 17 00:00:00 2001 From: miroslavpojer Date: Mon, 6 Apr 2026 18:26:28 +0200 Subject: [PATCH 13/20] Self-review and reduction of pylint exceptions. --- release_notes_generator/action_inputs.py | 8 +- .../chapters/custom_chapters.py | 144 +++++++------ .../data/utils/bulk_sub_issue_collector.py | 22 +- .../model/record/hierarchy_issue_record.py | 189 ++++++++++-------- .../chapters/test_custom_chapters.py | 185 +++++++++++++++++ .../model/test_hierarchy_issue_record.py | 111 ++++++++++ .../test_action_inputs.py | 57 ++++++ 7 files changed, 556 insertions(+), 160 deletions(-) diff --git a/release_notes_generator/action_inputs.py b/release_notes_generator/action_inputs.py index fd5aaf82..e346a84c 100644 --- a/release_notes_generator/action_inputs.py +++ b/release_notes_generator/action_inputs.py @@ -142,7 +142,7 @@ def get_from_tag_name() -> str: Get the from-tag name from the action inputs. """ raw = get_action_input(FROM_TAG_NAME, default="") - return normalize_version_tag(raw) # type: ignore[arg-type] + return normalize_version_tag(raw) @staticmethod def is_from_tag_name_defined() -> bool: @@ -185,8 +185,7 @@ def get_super_chapters() -> list[dict[str, Any]]: with a warning log. """ # Get the 'super-chapters' input from environment variables - super_chapters_input: str = get_action_input(SUPER_CHAPTERS, default="") # type: ignore[assignment] - # mypy: string is returned as default + super_chapters_input: str = get_action_input(SUPER_CHAPTERS, default="") # Parse the received string back to YAML array input. try: @@ -407,8 +406,7 @@ def get_print_empty_chapters() -> bool: """ Get the print empty chapters parameter value from the action inputs. """ - return get_action_input(PRINT_EMPTY_CHAPTERS, "true").lower() == "true" # type: ignore[union-attr] - # mypy: string is returned as default + return get_action_input(PRINT_EMPTY_CHAPTERS, "true").lower() == "true" @staticmethod def validate_input(input_value, expected_type: type, error_message: str, error_buffer: list) -> bool: diff --git a/release_notes_generator/chapters/custom_chapters.py b/release_notes_generator/chapters/custom_chapters.py index 56fb363c..26f02bd6 100644 --- a/release_notes_generator/chapters/custom_chapters.py +++ b/release_notes_generator/chapters/custom_chapters.py @@ -243,40 +243,32 @@ def _render_chapter_for_ids( finally: chapter.rows = original_rows - def _to_string_with_super_chapters(self) -> str: - """Render chapters grouped under super-chapter headings.""" - # Collect all record IDs claimed by at least one super chapter - all_super_labels: set[str] = set() - for sc in self._super_chapters: - all_super_labels.update(sc.labels) - - all_super_labels_list = list(all_super_labels) - - claimed_ids: set[str] = set() - for rid, labels in self._record_labels.items(): - if any(lbl in all_super_labels for lbl in labels): - claimed_ids.add(rid) - - result = "" - for sc in self._super_chapters: - matching_ids = self._collect_super_chapter_ids(sc) - - sc_block = "" - for chapter in self._sorted_chapters(): - if chapter.hidden: - continue - ch_str = self._render_chapter_for_ids(chapter, matching_ids, super_labels=sc.labels) - if ch_str: - sc_block += ch_str + "\n\n" - - if sc_block.strip(): - result += f"## {sc.title}\n{sc_block}" - elif self.print_empty_chapters: - result += f"## {sc.title}\nNo entries detected.\n\n" - - # Fallback: records not claimed by any super chapter. - # Hierarchy records that ARE claimed but have unmatched descendants also appear - # in Uncategorized with exclude_labels to render only the non-SC portion. + def _collect_claimed_ids(self, all_super_labels: set[str]) -> set[str]: + """Return record IDs whose labels intersect any super-chapter label.""" + return { + rid + for rid, labels in self._record_labels.items() + if any(lbl in all_super_labels for lbl in labels) + } + + def _render_super_chapter_block(self, sc: SuperChapter) -> str: + """Render all chapters filtered to the given super chapter's matching records.""" + matching_ids = self._collect_super_chapter_ids(sc) + sc_block = "" + for chapter in self._sorted_chapters(): + if chapter.hidden: + continue + ch_str = self._render_chapter_for_ids(chapter, matching_ids, super_labels=sc.labels) + if ch_str: + sc_block += ch_str + "\n\n" + if sc_block.strip(): + return f"## {sc.title}\n{sc_block}" + if self.print_empty_chapters: + return f"## {sc.title}\nNo entries detected.\n\n" + return "" + + def _collect_uncategorized_ids(self, claimed_ids: set[str], all_super_labels_list: list[str]) -> set[str]: + """Return IDs for records not claimed by any super chapter, plus partially-matched hierarchy IDs.""" unclaimed_ids: set[str] = set() partial_hierarchy_ids: set[str] = set() for chapter in self._sorted_chapters(): @@ -284,28 +276,69 @@ def _to_string_with_super_chapters(self) -> str: row_id_str = str(row_id) if row_id_str not in claimed_ids: unclaimed_ids.add(row_id_str) - elif row_id_str in claimed_ids: + else: record = self._records.get(row_id_str) if isinstance(record, HierarchyIssueRecord) and record.has_unmatched_descendants( all_super_labels_list ): partial_hierarchy_ids.add(row_id_str) + return unclaimed_ids | partial_hierarchy_ids - all_uncat_ids = unclaimed_ids | partial_hierarchy_ids - if all_uncat_ids: - uc_block = "" - for chapter in self._sorted_chapters(): - if chapter.hidden: - continue - ch_str = self._render_chapter_for_ids(chapter, all_uncat_ids, exclude_labels=all_super_labels_list) - if ch_str: - uc_block += ch_str + "\n\n" - if uc_block.strip(): - result += f"## {UNCATEGORIZED_CHAPTER_TITLE}\n{uc_block}" + def _render_uncategorized_block(self, all_uncat_ids: set[str], all_super_labels_list: list[str]) -> str: + """Render the Uncategorized section for records not claimed by any super chapter.""" + if not all_uncat_ids: + return "" + uc_block = "" + for chapter in self._sorted_chapters(): + if chapter.hidden: + continue + ch_str = self._render_chapter_for_ids(chapter, all_uncat_ids, exclude_labels=all_super_labels_list) + if ch_str: + uc_block += ch_str + "\n\n" + if uc_block.strip(): + return f"## {UNCATEGORIZED_CHAPTER_TITLE}\n{uc_block}" + return "" + def _to_string_with_super_chapters(self) -> str: + """Render chapters grouped under super-chapter headings.""" + all_super_labels: set[str] = {lbl for sc in self._super_chapters for lbl in sc.labels} + all_super_labels_list = list(all_super_labels) + claimed_ids = self._collect_claimed_ids(all_super_labels) + result = "".join(self._render_super_chapter_block(sc) for sc in self._super_chapters) + result += self._render_uncategorized_block( + self._collect_uncategorized_ids(claimed_ids, all_super_labels_list), + all_super_labels_list, + ) return result.strip() - def from_yaml_array(self, chapters: list[dict[str, Any]]) -> "CustomChapters": # type: ignore[override] + def _enforce_coh_constraint( + self, title: str, catch_open_hierarchy: bool, coh_chapter_title: str | None + ) -> tuple[bool, str | None]: + """Enforce the single catch-open-hierarchy chapter constraint. + + Parameters: + title: Chapter title being processed. + catch_open_hierarchy: Whether the current entry requests COH. + coh_chapter_title: Title of the first COH chapter seen so far, or None. + + Returns: + Updated (catch_open_hierarchy, coh_chapter_title) tuple. + """ + if not catch_open_hierarchy: + return False, coh_chapter_title + if coh_chapter_title is not None and coh_chapter_title != title: + logger.warning( + "Chapter '%s' has catch-open-hierarchy: true but '%s' already uses it; ignoring duplicate", + title, + coh_chapter_title, + ) + return False, coh_chapter_title + if coh_chapter_title is None: + return True, title + # else: same title repeating COH — silent merge pass-through + return True, coh_chapter_title + + def from_yaml_array(self, chapters: list[dict[str, Any]]) -> "CustomChapters": """ Populate the custom chapters from a list of dict objects. @@ -337,20 +370,9 @@ def from_yaml_array(self, chapters: list[dict[str, Any]]) -> "CustomChapters": catch_open_hierarchy = self._parse_bool_flag( title, chapter.get("catch-open-hierarchy", False), "catch-open-hierarchy" ) - - # Enforce single catch-open-hierarchy chapter constraint. - # Same-title repeats are treated as a merge and allowed through. - if catch_open_hierarchy: - if coh_chapter_title is not None and coh_chapter_title != title: - logger.warning( - "Chapter '%s' has catch-open-hierarchy: true but '%s' already uses it; ignoring duplicate", - title, - coh_chapter_title, - ) - catch_open_hierarchy = False - elif coh_chapter_title is None: - coh_chapter_title = title - # else: same title repeating COH — silent merge pass-through + catch_open_hierarchy, coh_chapter_title = self._enforce_coh_constraint( + title, catch_open_hierarchy, coh_chapter_title + ) unknown = set(chapter.keys()) - allowed_keys if unknown: diff --git a/release_notes_generator/data/utils/bulk_sub_issue_collector.py b/release_notes_generator/data/utils/bulk_sub_issue_collector.py index e51885e7..b73a69f6 100644 --- a/release_notes_generator/data/utils/bulk_sub_issue_collector.py +++ b/release_notes_generator/data/utils/bulk_sub_issue_collector.py @@ -24,8 +24,6 @@ import logging import time from dataclasses import dataclass -from typing import Dict, List, Optional, Set, Tuple - import requests from release_notes_generator.utils.record_utils import parse_issue_id, format_issue_id @@ -66,8 +64,8 @@ class BulkSubIssueCollector: def __init__( self, token: str, - cfg: Optional[CollectorConfig] = None, - session: Optional[requests.Session] = None, + cfg: CollectorConfig | None = None, + session: requests.Session | None = None, ): self._cfg = cfg or CollectorConfig() self._session = session or requests.Session() @@ -89,7 +87,7 @@ def scan_sub_issues_for_parents(self, parents_to_check: list[str]) -> list[str]: if not parents_to_check: return [] - new_parents_to_check: Set[str] = set() + new_parents_to_check: set[str] = set() self.parents_sub_issues = {} by_repo: dict[tuple[str, str], list[int]] = {} @@ -106,8 +104,8 @@ def scan_sub_issues_for_parents(self, parents_to_check: list[str]) -> list[str]: repo_chunk = repo_items[i : i + self._cfg.max_repos_per_request] # Maintain cursors per (org, repo, issue). - cursors: Dict[Tuple[str, str, int], Optional[str]] = {} - remaining_by_repo: Dict[Tuple[str, str], Set[int]] = {k: set(v) for k, v in repo_chunk} + cursors: dict[tuple[str, str, int], str | None] = {} + remaining_by_repo: dict[tuple[str, str], set[int]] = {k: set(v) for k, v in repo_chunk} for (org, repo), nums in remaining_by_repo.items(): for n in nums: cursors[(org, repo, n)] = None @@ -116,14 +114,14 @@ def scan_sub_issues_for_parents(self, parents_to_check: list[str]) -> list[str]: while any(remaining_by_repo.values()): # Build one GraphQL query with up to max_repos_per_request repos, # each with up to max_parents_per_repo parent issues that still have pages. - repo_blocks: List[str] = [] - alias_maps: Dict[str, Tuple[str, str, int]] = {} # alias -> (org, repo, parent_num) + repo_blocks: list[str] = [] + alias_maps: dict[str, tuple[str, str, int]] = {} # alias -> (org, repo, parent_num) for r_idx, ((org, repo), parents_rem) in enumerate(remaining_by_repo.items()): if not parents_rem: continue current_parents = list(parents_rem)[: self._cfg.max_parents_per_repo] - issue_blocks: List[str] = [] + issue_blocks: list[str] = [] for p_idx, parent_num in enumerate(current_parents): alias = f"i{r_idx}_{p_idx}" alias_maps[alias] = (org, repo, parent_num) @@ -210,7 +208,7 @@ def scan_sub_issues_for_parents(self, parents_to_check: list[str]) -> list[str]: # ---------- internals ---------- def _post_graphql(self, payload: dict) -> dict: - last_exc: Optional[Exception] = None + last_exc: Exception | None = None for attempt in range(1, self._cfg.max_retries + 1): try: logger.debug("Posting graphql query") @@ -246,7 +244,7 @@ def _post_graphql(self, payload: dict) -> dict: raise last_exc raise RuntimeError("GraphQL POST failed without exception.") - def _find_alias_node(self, repo_block: dict, alias: str) -> Optional[dict]: + def _find_alias_node(self, repo_block: dict, alias: str) -> dict | None: """ Given top-level 'data' (mapping of repo aliases -> repo object), return the 'issue' object under whichever repo alias contains our issue alias. diff --git a/release_notes_generator/model/record/hierarchy_issue_record.py b/release_notes_generator/model/record/hierarchy_issue_record.py index 7aec777f..b29481a3 100644 --- a/release_notes_generator/model/record/hierarchy_issue_record.py +++ b/release_notes_generator/model/record/hierarchy_issue_record.py @@ -19,7 +19,7 @@ """ import logging -from typing import Optional, Any +from typing import Any from github.Issue import Issue from release_notes_generator.action_inputs import ActionInputs @@ -36,7 +36,7 @@ class HierarchyIssueRecord(IssueRecord): Inherits from IssueRecord and provides additional functionality specific to issues. """ - def __init__(self, issue: Issue, issue_labels: Optional[list[str]] = None, skip: bool = False): + def __init__(self, issue: Issue, issue_labels: list[str] | None = None, skip: bool = False): super().__init__(issue, issue_labels, skip) self._level: int = 0 @@ -93,6 +93,7 @@ def progress(self) -> str: @property def developers(self) -> list[str]: + """Unique, sorted list of developers across this issue and all descendants.""" issue = self._issue if not issue: return [] @@ -116,6 +117,7 @@ def developers(self) -> list[str]: return sorted(devs) def pull_requests_count(self) -> int: + """Return the total number of pull requests across this issue and all descendants.""" count = super().pull_requests_count() for sub_issue in self._sub_issues.values(): @@ -160,6 +162,7 @@ def contains_change_increment(self) -> bool: return False def get_labels(self) -> list[str]: + """Return all labels from this issue, its sub-issues, sub-hierarchy-issues, and attached PRs.""" labels: set[str] = set() labels.update(label.name for label in self._issue.get_labels()) @@ -217,51 +220,62 @@ def has_unmatched_descendants(self, all_super_labels: list[str]) -> bool: return False # methods - override ancestor methods - def to_chapter_row( - self, - add_into_chapters: bool = True, - label_filter: list[str] | None = None, - exclude_labels: list[str] | None = None, - ) -> str: - logger.debug("Rendering hierarchy issue row for issue #%s", self.issue.number) - row_prefix = f"{ActionInputs.get_duplicity_icon()} " if self.chapter_presence_count() > 1 else "" - format_values: dict[str, Any] = {} - # collect format values + def _collect_format_values(self) -> dict[str, Any]: + """Collect template substitution values for the hierarchy issue row format string.""" + format_values: dict[str, Any] = {} format_values["number"] = f"#{self.issue.number}" format_values["title"] = self.issue.title format_values["author"] = self.author format_values["assignees"] = ", ".join(self.assignees) format_values["developers"] = ", ".join(self.developers) - if self.issue_type is not None: - format_values["type"] = self.issue_type - else: - format_values["type"] = "" + format_values["type"] = self.issue_type if self.issue_type is not None else "" format_values["progress"] = self.progress - list_pr_links = self.get_pr_links() - if len(list_pr_links) > 0: - format_values["pull-requests"] = ", ".join(list_pr_links) - else: - format_values["pull-requests"] = "" - - indent: str = " " * self._level - if self._level > 0: - indent += "- " - - # create first issue row - row = f"{indent}{row_prefix}" + format_row_with_suppression( - ActionInputs.get_row_format_hierarchy_issue(), format_values - ) + format_values["pull-requests"] = ", ".join(list_pr_links) if list_pr_links else "" + return format_values + + def _append_release_notes_block(self, row: str) -> str: + """Append an indented Release Notes section to *row* when present.""" + if not self.contains_release_notes(): + return row + sub_indent: str = " " * (self._level + 1) + row = f"{row}\n{sub_indent}- _Release Notes_:" + rls_block = "\n".join(f"{sub_indent}{line}" if line else "" for line in self.get_rls_notes().splitlines()) + return f"{row}\n{rls_block}" + + def _build_open_sub_hierarchy_row( + self, + sub_hierarchy_issue: "HierarchyIssueRecord", + label_filter: list[str] | None, + exclude_labels: list[str] | None, + ) -> str: + """Render an open sub-hierarchy issue row and insert the open-hierarchy icon after the list marker. - # add extra section with release notes if detected - if self.contains_release_notes(): - sub_indent: str = " " * (self._level + 1) - row = f"{row}\n{sub_indent}- _Release Notes_:" - rls_block = "\n".join(f"{sub_indent}{line}" if line else "" for line in self.get_rls_notes().splitlines()) - row = f"{row}\n{rls_block}" + Highlight open children under a closed parent to signal incomplete work. + Insert icon after the list marker ('- ') to preserve Markdown structure, + and skip insertion entirely when the icon is empty. + """ + sub_row = sub_hierarchy_issue.to_chapter_row(label_filter=label_filter, exclude_labels=exclude_labels) + icon = ActionInputs.get_open_hierarchy_sub_issue_icon() + if not icon: + return sub_row + header_line, newline, remaining_lines = sub_row.partition("\n") + header_text = header_line.lstrip() + spaces = header_line[: len(header_line) - len(header_text)] + if header_text.startswith("- "): + marker, content = "- ", header_text[2:] + else: + marker, content = "", header_text + return f"{spaces}{marker}{icon} {content}{newline}{remaining_lines}" - # add sub-hierarchy issues + def _append_sub_hierarchy_rows( + self, + row: str, + label_filter: list[str] | None, + exclude_labels: list[str] | None, + ) -> str: + """Append rendered rows for all qualifying sub-hierarchy issues to *row*.""" for sub_hierarchy_issue in sorted(self._sub_hierarchy_issues.values(), key=lambda r: r.issue.number): logger.debug("Rendering sub-hierarchy issue row for #%s", sub_hierarchy_issue.issue.number) if label_filter and not sub_hierarchy_issue.has_matching_labels(label_filter): @@ -272,59 +286,70 @@ def to_chapter_row( and sub_hierarchy_issue.has_matching_labels(exclude_labels) ): continue - if self.is_open: - if not sub_hierarchy_issue.contains_change_increment(): - continue + if self.is_open and not sub_hierarchy_issue.contains_change_increment(): + continue # Closed parent: render all sub-hierarchy issues regardless of state or change increment logger.debug("Rendering sub-hierarchy issue #%s", sub_hierarchy_issue.issue.number) if self.is_closed and sub_hierarchy_issue.is_open: - sub_row = sub_hierarchy_issue.to_chapter_row(label_filter=label_filter, exclude_labels=exclude_labels) - # Highlight open children under a closed parent to signal incomplete work. - # Insert icon after the list marker ('- ') to preserve Markdown structure, - # and skip insertion entirely when the icon is empty. - icon = ActionInputs.get_open_hierarchy_sub_issue_icon() - if icon: - header_line, newline, remaining_lines = sub_row.partition("\n") - header_text = header_line.lstrip() - spaces = header_line[: len(header_line) - len(header_text)] - if header_text.startswith("- "): - marker, content = "- ", header_text[2:] - else: - marker, content = "", header_text - sub_row = f"{spaces}{marker}{icon} {content}{newline}{remaining_lines}" + sub_row = self._build_open_sub_hierarchy_row(sub_hierarchy_issue, label_filter, exclude_labels) else: sub_row = sub_hierarchy_issue.to_chapter_row(label_filter=label_filter, exclude_labels=exclude_labels) row = f"{row}\n{sub_row}" + return row - # add sub-issues - if len(self._sub_issues) > 0: - sub_indent = " " * (self._level + 1) - for sub_issue in sorted(self._sub_issues.values(), key=lambda r: r.issue.number): - logger.debug("Rendering sub-issue row for issue #%s", sub_issue.issue.number) - if label_filter and not any(lbl in label_filter for lbl in sub_issue.labels): - continue - if exclude_labels and any(lbl in exclude_labels for lbl in sub_issue.labels): - continue - if self.is_open: - if sub_issue.is_open: - continue # only closed issues are reported in release notes - if not sub_issue.contains_change_increment(): - continue # skip sub-issues without change increment - # Closed parent: render all sub-issues regardless of state or change increment - - logger.debug("Rendering sub-issue #%s", sub_issue.issue.number) - open_icon_prefix = "" - if self.is_closed and sub_issue.is_open: - # Highlight open children under a closed parent to signal incomplete work - open_icon_prefix = ActionInputs.get_open_hierarchy_sub_issue_icon() + " " - sub_issue_block = "- " + open_icon_prefix + sub_issue.to_chapter_row() - ind_child_block = "\n".join( - f"{sub_indent}{line}" if line else "" for line in sub_issue_block.splitlines() - ) - row = f"{row}\n{ind_child_block}" - # else: this will be reported in service chapters as violation of hierarchy in this initial version - # No data loss - in service chapter there will be all detail not presented here + def _append_sub_issue_rows( + self, + row: str, + label_filter: list[str] | None, + exclude_labels: list[str] | None, + ) -> str: + """Append rendered rows for all qualifying direct sub-issues to *row*.""" + if not self._sub_issues: + # else: this will be reported in service chapters as violation of hierarchy in this initial version + # No data loss - in service chapter there will be all detail not presented here + return row + sub_indent = " " * (self._level + 1) + for sub_issue in sorted(self._sub_issues.values(), key=lambda r: r.issue.number): + logger.debug("Rendering sub-issue row for issue #%s", sub_issue.issue.number) + if label_filter and not any(lbl in label_filter for lbl in sub_issue.labels): + continue + if exclude_labels and any(lbl in exclude_labels for lbl in sub_issue.labels): + continue + if self.is_open: + if sub_issue.is_open: + continue # only closed issues are reported in release notes + if not sub_issue.contains_change_increment(): + continue # skip sub-issues without change increment + # Closed parent: render all sub-issues regardless of state or change increment + logger.debug("Rendering sub-issue #%s", sub_issue.issue.number) + open_icon_prefix = "" + if self.is_closed and sub_issue.is_open: + # Highlight open children under a closed parent to signal incomplete work + open_icon_prefix = ActionInputs.get_open_hierarchy_sub_issue_icon() + " " + sub_issue_block = "- " + open_icon_prefix + sub_issue.to_chapter_row() + ind_child_block = "\n".join( + f"{sub_indent}{line}" if line else "" for line in sub_issue_block.splitlines() + ) + row = f"{row}\n{ind_child_block}" + return row + def to_chapter_row( + self, + add_into_chapters: bool = True, + label_filter: list[str] | None = None, + exclude_labels: list[str] | None = None, + ) -> str: + logger.debug("Rendering hierarchy issue row for issue #%s", self.issue.number) + row_prefix = f"{ActionInputs.get_duplicity_icon()} " if self.chapter_presence_count() > 1 else "" + indent: str = " " * self._level + if self._level > 0: + indent += "- " + row = f"{indent}{row_prefix}" + format_row_with_suppression( + ActionInputs.get_row_format_hierarchy_issue(), self._collect_format_values() + ) + row = self._append_release_notes_block(row) + row = self._append_sub_hierarchy_rows(row, label_filter, exclude_labels) + row = self._append_sub_issue_rows(row, label_filter, exclude_labels) return row def order_hierarchy_levels(self, level: int = 0) -> None: diff --git a/tests/unit/release_notes_generator/chapters/test_custom_chapters.py b/tests/unit/release_notes_generator/chapters/test_custom_chapters.py index ccc6df82..d9fcf188 100644 --- a/tests/unit/release_notes_generator/chapters/test_custom_chapters.py +++ b/tests/unit/release_notes_generator/chapters/test_custom_chapters.py @@ -1838,3 +1838,188 @@ def test_sc_combo_c6_plain_record_routing_unchanged(mocker, record_stub): assert "Uncategorized" in output uncat_section = output.split("Uncategorized")[1] assert "org/repo#11 row" in uncat_section + + +def test_from_yaml_array_coh_chapter_with_explicit_null_labels(): + """catch-open-hierarchy chapter with 'labels: null' is accepted with an empty label list.""" + cc = CustomChapters() + cc.from_yaml_array([{"title": "Open Issues", "catch-open-hierarchy": True, "labels": None}]) + assert "Open Issues" in cc.chapters + assert cc.chapters["Open Issues"].catch_open_hierarchy is True + assert cc.chapters["Open Issues"].labels == [] + + +def test_from_yaml_array_chapter_order_conflict_warns(caplog): + """Same chapter title defined twice with different orders logs a warning and keeps the first.""" + caplog.set_level("WARNING", logger="release_notes_generator.chapters.custom_chapters") + cc = CustomChapters() + cc.from_yaml_array( + [ + {"title": "Changes", "labels": "bug", "order": 1}, + {"title": "Changes", "labels": "enhancement", "order": 2}, + ] + ) + assert "Changes" in cc.chapters + assert cc.chapters["Changes"].order == 1 + assert any("conflicting order" in r.message for r in caplog.records) + + +def test_from_yaml_array_repeated_chapter_with_hidden_logs_info(caplog): + """Repeated chapter definition with hidden=True logs an info message about hidden status.""" + caplog.set_level("INFO", logger="release_notes_generator.chapters.custom_chapters") + cc = CustomChapters() + cc.from_yaml_array( + [ + {"title": "Changes", "labels": "bug"}, + {"title": "Changes", "labels": "enhancement", "hidden": True}, + ] + ) + assert "Changes" in cc.chapters + assert any("marked as hidden" in r.message for r in caplog.records) + + +def test_super_chapters_hidden_chapter_skipped_in_sc_and_uncat_loops(mocker, record_stub): + """Hidden chapters are skipped during both SC-block and Uncategorized-block rendering.""" + cc = make_super_chapters_cc( + mocker, + [ + {"title": "SC-Chapter", "labels": "sc-label"}, + {"title": "Hidden-Chapter", "labels": "other-label", "hidden": True}, + ], + [{"title": "Security", "labels": ["sc-label"]}], + print_empty=True, + ) + r_sc = record_stub("org/repo#1", ["sc-label"]) + r_plain = record_stub("org/repo#2", ["other-label"]) # assigned to hidden chapter + cc.populate({"org/repo#1": r_sc, "org/repo#2": r_plain}) + + output = cc.to_string() + + # SC section renders with visible chapter (hidden chapter is skipped in SC loop) + assert "## Security" in output + assert "org/repo#1 row" in output + # Uncategorized section is entered (r_plain is unclaimed) but hidden chapter is skipped, + # so r_plain does NOT appear in the rendered output + assert "Uncategorized" in output + assert "org/repo#2 row" not in output + + +def test_super_chapters_empty_sc_block_printed_when_all_chapters_hidden(mocker, record_stub): + """When all chapters are hidden, a super chapter with no visible content renders 'No entries detected.'""" + cc = make_super_chapters_cc( + mocker, + [{"title": "Hidden-Only", "labels": "bug", "hidden": True}], + [{"title": "Security", "labels": ["sc-label"]}], + print_empty=True, + ) + r1 = record_stub("org/repo#1", ["bug", "sc-label"]) + cc.populate({"org/repo#1": r1}) + + output = cc.to_string() + + assert "## Security" in output + assert "No entries detected." in output + + +def test_render_chapter_for_ids_hierarchy_record_no_matching_labels_skipped(mocker, record_stub): + """HierarchyIssueRecord whose has_matching_labels returns False is skipped in SC rendering.""" + cc = make_super_chapters_cc( + mocker, + [{"title": "Changes", "labels": "sc-label"}], + [{"title": "Security", "labels": ["sc-label"]}], + print_empty=False, + ) + mock_hi = mocker.create_autospec(HierarchyIssueRecord, instance=True) + mock_hi.labels = ["sc-label"] + mock_hi.get_labels.return_value = ["sc-label"] + mock_hi.contains_change_increment.return_value = True + mock_hi.skip = False + mock_hi.is_open = True + mock_hi.has_matching_labels.return_value = False + mock_hi.to_chapter_row.return_value = "hi_row" + cc.populate({"org/repo#1": mock_hi}) + + output = cc.to_string() + + # The record must NOT appear in Security section because has_matching_labels returned False + security_section = output.split("## Security")[1].split("## ")[0] if "## Security" in output else "" + assert "hi_row" not in security_section + + +# --- _enforce_coh_constraint via from_yaml_array --- + + +def test_enforce_coh_constraint_no_coh_passes_through(caplog): + """Chapter with no catch-open-hierarchy is registered normally (pass-through path).""" + caplog.set_level("WARNING", logger="release_notes_generator.chapters.custom_chapters") + cc = CustomChapters() + cc.from_yaml_array([{"title": "Features", "labels": "feature"}]) + assert "Features" in cc.chapters + assert cc.chapters["Features"].catch_open_hierarchy is False + assert not caplog.records + + +def test_enforce_coh_constraint_first_coh_chapter_accepted(): + """First chapter with catch-open-hierarchy is accepted without warning.""" + cc = CustomChapters() + cc.from_yaml_array([{"title": "Open Issues", "catch-open-hierarchy": True}]) + assert "Open Issues" in cc.chapters + assert cc.chapters["Open Issues"].catch_open_hierarchy is True + + +def test_enforce_coh_constraint_duplicate_different_title_rejected_with_warning(caplog): + """Second distinct title with catch-open-hierarchy is demoted and a warning is emitted.""" + caplog.set_level("WARNING", logger="release_notes_generator.chapters.custom_chapters") + cc = CustomChapters() + cc.from_yaml_array( + [ + {"title": "First", "catch-open-hierarchy": True}, + {"title": "Second", "catch-open-hierarchy": True, "labels": "bug"}, + ] + ) + assert cc.chapters["First"].catch_open_hierarchy is True + assert cc.chapters["Second"].catch_open_hierarchy is False + assert any("ignoring duplicate" in r.message.lower() for r in caplog.records) + + +def test_enforce_coh_constraint_same_title_repeat_merges_silently(caplog): + """Repeating catch-open-hierarchy on the same title merges labels without warning.""" + caplog.set_level("WARNING", logger="release_notes_generator.chapters.custom_chapters") + cc = CustomChapters() + cc.from_yaml_array( + [ + {"title": "Open Issues", "catch-open-hierarchy": True, "labels": "epic"}, + {"title": "Open Issues", "catch-open-hierarchy": True, "labels": "feature"}, + ] + ) + assert cc.chapters["Open Issues"].catch_open_hierarchy is True + assert cc.chapters["Open Issues"].labels == ["epic", "feature"] + assert not any("ignoring duplicate" in r.message.lower() for r in caplog.records) + + +# --- _collect_uncategorized_ids via to_string with super chapters --- + + +def test_collect_uncategorized_ids_claimed_hierarchy_with_all_matched_not_in_uncat(mocker, record_stub): + """A hierarchy record whose every descendant is SC-matched must not appear in Uncategorized.""" + cc = make_super_chapters_cc( + mocker, + [{"title": "Features", "label": "enhancement"}], + [{"title": "Security", "labels": ["scope:security"]}], + ) + r1 = record_stub("org/repo#1", ["enhancement", "scope:security"]) + cc.populate({"org/repo#1": r1}) + output = cc.to_string() + assert "Uncategorized" not in output, f"Claimed record must not appear in Uncategorized; got:\n{output}" + + +def test_collect_uncategorized_ids_no_rows_produces_no_uncat_section(mocker): + """When all chapters are empty, no Uncategorized section is rendered.""" + cc = make_super_chapters_cc( + mocker, + [{"title": "Features", "label": "feature"}], + [{"title": "Security", "labels": ["scope:security"]}], + print_empty=False, + ) + output = cc.to_string() + assert "Uncategorized" not in output diff --git a/tests/unit/release_notes_generator/model/test_hierarchy_issue_record.py b/tests/unit/release_notes_generator/model/test_hierarchy_issue_record.py index 232575f6..ff201be1 100644 --- a/tests/unit/release_notes_generator/model/test_hierarchy_issue_record.py +++ b/tests/unit/release_notes_generator/model/test_hierarchy_issue_record.py @@ -912,3 +912,114 @@ def test_five_level_hierarchy_label_filter_and_exclude_labels(mocker, patch_hier assert "#4" in row_plain, f"Story must appear (has non-SC path); got:\n{row_plain}" assert "#11" in row_plain, f"Plain task must appear; got:\n{row_plain}" assert "#10" not in row_plain, f"SC task must be excluded; got:\n{row_plain}" + + +def test_developers_returns_empty_list_when_issue_is_none(): + """developers returns [] when the underlying issue is None (defensive guard).""" + record = HierarchyIssueRecord(None) # type: ignore[arg-type] + assert record.developers == [] + + +def test_pull_requests_count_cross_repo_sub_issue(mocker, make_hierarchy_issue): + """A cross-repo sub-issue counts as 1 PR regardless of its own pull-request list.""" + parent = HierarchyIssueRecord(make_hierarchy_issue(100, IssueRecord.ISSUE_STATE_OPEN)) + sub = SubIssueRecord(make_hierarchy_issue(200, IssueRecord.ISSUE_STATE_CLOSED)) + sub.is_cross_repo = True + parent.sub_issues["org/repo#200"] = sub + + assert parent.pull_requests_count() == 1 + + +def test_pull_requests_count_cross_repo_sub_hierarchy_issue(mocker, make_hierarchy_issue): + """A cross-repo sub-hierarchy-issue counts as 1 PR.""" + parent = HierarchyIssueRecord(make_hierarchy_issue(100, IssueRecord.ISSUE_STATE_OPEN)) + child = HierarchyIssueRecord(make_hierarchy_issue(200, IssueRecord.ISSUE_STATE_CLOSED)) + child.is_cross_repo = True + parent.sub_hierarchy_issues["org/repo#200"] = child + + assert parent.pull_requests_count() == 1 + + +def test_get_labels_includes_pr_labels(mocker, make_hierarchy_issue, make_sub_issue): + """get_labels aggregates labels from attached pull requests.""" + issue = make_hierarchy_issue(10, IssueRecord.ISSUE_STATE_OPEN) + record = HierarchyIssueRecord(issue) + + pr = mocker.Mock() + mock_label = mocker.Mock() + mock_label.name = "pr-label" + pr.get_labels.return_value = [mock_label] + record.register_pull_request(pr) + + labels = record.get_labels() + assert "pr-label" in labels + + +def test_has_unmatched_descendants_leaf_hierarchy_child_unmatched(mocker, make_hierarchy_issue): + """Leaf sub-hierarchy-issue whose own labels don't match the SC set returns True.""" + parent = HierarchyIssueRecord(make_hierarchy_issue(1, IssueRecord.ISSUE_STATE_OPEN)) + + leaf_issue = make_hierarchy_issue(2, IssueRecord.ISSUE_STATE_OPEN) + other_label = mocker.Mock() + other_label.name = "other-label" + leaf_issue.get_labels.return_value = [other_label] + leaf = HierarchyIssueRecord(leaf_issue) + parent.sub_hierarchy_issues["org/repo#2"] = leaf + + assert parent.has_unmatched_descendants(["sc-label"]) is True + + +def test_has_unmatched_descendants_leaf_hierarchy_child_matched(mocker, make_hierarchy_issue): + """Leaf sub-hierarchy-issue whose labels match the SC set returns False.""" + parent = HierarchyIssueRecord(make_hierarchy_issue(1, IssueRecord.ISSUE_STATE_OPEN)) + + leaf_issue = make_hierarchy_issue(2, IssueRecord.ISSUE_STATE_OPEN) + sc_label = mocker.Mock() + sc_label.name = "sc-label" + leaf_issue.get_labels.return_value = [sc_label] + leaf = HierarchyIssueRecord(leaf_issue) + parent.sub_hierarchy_issues["org/repo#2"] = leaf + + assert parent.has_unmatched_descendants(["sc-label"]) is False + + +# --- release notes block rendering --- + + +def test_to_chapter_row_renders_release_notes_heading_and_content(mocker, patch_hierarchy_action_inputs): + """Issue body with a Release Notes section is rendered as an indented block by to_chapter_row().""" + parent = make_minimal_issue(mocker, IssueRecord.ISSUE_STATE_CLOSED, number=300) + parent.body = "Description\nRelease Notes:\n- Fixed the critical bug" + record = HierarchyIssueRecord(parent) + + row = record.to_chapter_row() + + assert "_Release Notes_:" in row, f"Release Notes heading must appear; got:\n{row}" + assert "Fixed the critical bug" in row, f"Release notes content must appear; got:\n{row}" + + +def test_to_chapter_row_release_notes_block_indented_relative_to_level(mocker, patch_hierarchy_action_inputs): + """Release Notes heading is indented one level deeper than the issue's own hierarchy level.""" + parent = make_minimal_issue(mocker, IssueRecord.ISSUE_STATE_CLOSED, number=300) + parent.body = "Description\nRelease Notes:\n- Fixed the critical bug" + record = HierarchyIssueRecord(parent) + record.level = 0 + + row = record.to_chapter_row() + + rls_line = next((line for line in row.splitlines() if "_Release Notes_:" in line), None) + assert rls_line is not None, f"Release Notes heading line missing; got:\n{row}" + assert rls_line.startswith(" - "), ( + f"At level 0 heading must start with ' - ' (2-space indent + list marker); got: {rls_line!r}" + ) + + +def test_to_chapter_row_no_release_notes_body_omits_block(mocker, patch_hierarchy_action_inputs): + """Issue body without a Release Notes section produces no Release Notes heading.""" + parent = make_minimal_issue(mocker, IssueRecord.ISSUE_STATE_CLOSED, number=301) + parent.body = "Just a plain description with no special section." + record = HierarchyIssueRecord(parent) + + row = record.to_chapter_row() + + assert "_Release Notes_:" not in row, f"No Release Notes heading expected; got:\n{row}" diff --git a/tests/unit/release_notes_generator/test_action_inputs.py b/tests/unit/release_notes_generator/test_action_inputs.py index 1bce9cc8..c97eb6fe 100644 --- a/tests/unit/release_notes_generator/test_action_inputs.py +++ b/tests/unit/release_notes_generator/test_action_inputs.py @@ -61,6 +61,7 @@ ("get_duplicity_icon", "Oj", "Duplicity icon must be a non-empty string and have a length of 1."), ("get_row_format_issue", "", "Issue row format must be a non-empty string."), ("get_row_format_pr", "", "PR Row format must be a non-empty string."), + ("get_row_format_hierarchy_issue", "", "Hierarchy Issue row format must be a non-empty string."), ("get_release_notes_title", "", "Release Notes title must be a non-empty string and have non-zero length."), ( "get_coderabbit_release_notes_title", @@ -588,6 +589,62 @@ def test_get_row_format_link_pr_false(mocker): assert ActionInputs.get_row_format_link_pr() is False +def test_get_github_owner_no_slash_in_repository_id(monkeypatch, mocker): + """Repository ID without '/' sets owner to the whole string.""" + monkeypatch.setattr(ActionInputs, "_owner", "") + mocker.patch("release_notes_generator.action_inputs.get_action_input", return_value="standalone") + assert ActionInputs.get_github_owner() == "standalone" + + +def test_get_github_owner_with_slash_in_repository_id(monkeypatch, mocker): + """Repository ID with '/' extracts the owner part before the slash.""" + monkeypatch.setattr(ActionInputs, "_owner", "") + mocker.patch("release_notes_generator.action_inputs.get_action_input", return_value="my-org/my-repo") + assert ActionInputs.get_github_owner() == "my-org" + + +def test_get_github_owner_cache_hit(monkeypatch): + """Cached owner value is returned without querying the environment.""" + monkeypatch.setattr(ActionInputs, "_owner", "primed-org") + assert ActionInputs.get_github_owner() == "primed-org" + + +def test_get_github_repo_name_no_slash_in_repository_id(monkeypatch, mocker): + """Repository ID without '/' sets repo_name to the whole string.""" + monkeypatch.setattr(ActionInputs, "_repo_name", "") + mocker.patch("release_notes_generator.action_inputs.get_action_input", return_value="standalone") + assert ActionInputs.get_github_repo_name() == "standalone" + + +def test_get_github_repo_name_with_slash_in_repository_id(monkeypatch, mocker): + """Repository ID with '/' extracts the repo name part after the slash.""" + monkeypatch.setattr(ActionInputs, "_repo_name", "") + mocker.patch("release_notes_generator.action_inputs.get_action_input", return_value="my-org/my-repo") + assert ActionInputs.get_github_repo_name() == "my-repo" + + +def test_get_github_repo_name_cache_hit(monkeypatch): + """Cached repo name is returned without querying the environment.""" + monkeypatch.setattr(ActionInputs, "_repo_name", "primed-repo") + assert ActionInputs.get_github_repo_name() == "primed-repo" + + +def test_get_open_hierarchy_sub_issue_icon_default(): + assert ActionInputs.get_open_hierarchy_sub_issue_icon() == "🟡" + + +def test_get_open_hierarchy_sub_issue_icon_custom(mocker): + mocker.patch("release_notes_generator.action_inputs.get_action_input", return_value="⚡") + assert ActionInputs.get_open_hierarchy_sub_issue_icon() == "⚡" + + +def test_detect_row_format_invalid_keywords_unknown_row_type(caplog): + """Unknown row_type logs a warning and defaults to Issue keys.""" + caplog.set_level("WARNING", logger="release_notes_generator.action_inputs") + ActionInputs._detect_row_format_invalid_keywords("{number} {bogus}", row_type="Unknown") + assert any("Unknown row_type" in r.message for r in caplog.records) + + # Mirrored test file for release_notes_generator/generator.py # Extracted from previous aggregated test_release_notes_generator.py From 69d5b2dba1b9014fac6eb1b4fdfc0ed0c9461049 Mon Sep 17 00:00:00 2001 From: miroslavpojer Date: Mon, 6 Apr 2026 18:27:31 +0200 Subject: [PATCH 14/20] Fixed black. --- release_notes_generator/chapters/custom_chapters.py | 6 +----- .../model/record/hierarchy_issue_record.py | 4 +--- 2 files changed, 2 insertions(+), 8 deletions(-) diff --git a/release_notes_generator/chapters/custom_chapters.py b/release_notes_generator/chapters/custom_chapters.py index 26f02bd6..1956a452 100644 --- a/release_notes_generator/chapters/custom_chapters.py +++ b/release_notes_generator/chapters/custom_chapters.py @@ -245,11 +245,7 @@ def _render_chapter_for_ids( def _collect_claimed_ids(self, all_super_labels: set[str]) -> set[str]: """Return record IDs whose labels intersect any super-chapter label.""" - return { - rid - for rid, labels in self._record_labels.items() - if any(lbl in all_super_labels for lbl in labels) - } + return {rid for rid, labels in self._record_labels.items() if any(lbl in all_super_labels for lbl in labels)} def _render_super_chapter_block(self, sc: SuperChapter) -> str: """Render all chapters filtered to the given super chapter's matching records.""" diff --git a/release_notes_generator/model/record/hierarchy_issue_record.py b/release_notes_generator/model/record/hierarchy_issue_record.py index b29481a3..a7c6aa7e 100644 --- a/release_notes_generator/model/record/hierarchy_issue_record.py +++ b/release_notes_generator/model/record/hierarchy_issue_record.py @@ -327,9 +327,7 @@ def _append_sub_issue_rows( # Highlight open children under a closed parent to signal incomplete work open_icon_prefix = ActionInputs.get_open_hierarchy_sub_issue_icon() + " " sub_issue_block = "- " + open_icon_prefix + sub_issue.to_chapter_row() - ind_child_block = "\n".join( - f"{sub_indent}{line}" if line else "" for line in sub_issue_block.splitlines() - ) + ind_child_block = "\n".join(f"{sub_indent}{line}" if line else "" for line in sub_issue_block.splitlines()) row = f"{row}\n{ind_child_block}" return row From 4666b0da0c045517fdb6331022cd3902a3cc43b2 Mon Sep 17 00:00:00 2001 From: miroslavpojer Date: Mon, 6 Apr 2026 18:35:07 +0200 Subject: [PATCH 15/20] refactor: update type hints in CustomChapters and HierarchyIssueRecord classes for clarity --- release_notes_generator/chapters/custom_chapters.py | 3 ++- .../model/record/hierarchy_issue_record.py | 8 +++----- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/release_notes_generator/chapters/custom_chapters.py b/release_notes_generator/chapters/custom_chapters.py index 1956a452..3b67670c 100644 --- a/release_notes_generator/chapters/custom_chapters.py +++ b/release_notes_generator/chapters/custom_chapters.py @@ -307,8 +307,9 @@ def _to_string_with_super_chapters(self) -> str: ) return result.strip() + @staticmethod def _enforce_coh_constraint( - self, title: str, catch_open_hierarchy: bool, coh_chapter_title: str | None + title: str, catch_open_hierarchy: bool, coh_chapter_title: str | None ) -> tuple[bool, str | None]: """Enforce the single catch-open-hierarchy chapter constraint. diff --git a/release_notes_generator/model/record/hierarchy_issue_record.py b/release_notes_generator/model/record/hierarchy_issue_record.py index a7c6aa7e..d0c4abf4 100644 --- a/release_notes_generator/model/record/hierarchy_issue_record.py +++ b/release_notes_generator/model/record/hierarchy_issue_record.py @@ -19,7 +19,6 @@ """ import logging -from typing import Any from github.Issue import Issue from release_notes_generator.action_inputs import ActionInputs @@ -221,9 +220,9 @@ def has_unmatched_descendants(self, all_super_labels: list[str]) -> bool: # methods - override ancestor methods - def _collect_format_values(self) -> dict[str, Any]: + def _collect_format_values(self) -> dict[str, str]: """Collect template substitution values for the hierarchy issue row format string.""" - format_values: dict[str, Any] = {} + format_values: dict[str, str] = {} format_values["number"] = f"#{self.issue.number}" format_values["title"] = self.issue.title format_values["author"] = self.author @@ -305,8 +304,7 @@ def _append_sub_issue_rows( ) -> str: """Append rendered rows for all qualifying direct sub-issues to *row*.""" if not self._sub_issues: - # else: this will be reported in service chapters as violation of hierarchy in this initial version - # No data loss - in service chapter there will be all detail not presented here + # No sub-issues: violations of hierarchy are reported in service chapters (no data loss) return row sub_indent = " " * (self._level + 1) for sub_issue in sorted(self._sub_issues.values(), key=lambda r: r.issue.number): From 2514d44853dfb4f0914edd026b7c21bd8307b560 Mon Sep 17 00:00:00 2001 From: miroslavpojer Date: Mon, 6 Apr 2026 18:57:56 +0200 Subject: [PATCH 16/20] Fix review notes. --- .github/copilot-instructions.md | 12 ++++++------ release_notes_generator/action_inputs.py | 3 +-- .../model/record/hierarchy_issue_record.py | 5 ++++- tests/unit/conftest.py | 3 --- .../release_notes_generator/test_action_inputs.py | 10 ++++++++++ 5 files changed, 21 insertions(+), 12 deletions(-) diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index 96e3170b..12dccd31 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -85,12 +85,12 @@ Testing - Must place shared test helper functions and factory fixtures in the nearest `conftest.py` and reuse them across tests. - Must annotate pytest fixture parameters with `MockerFixture` (from `pytest_mock`) and return types with `Callable[..., T]` (from `collections.abc`) when the fixture returns a factory function. - Prefer TDD workflow: - 1. Create or update `SPEC.md` in the relevant package directory to describe the expected solution — list the scenarios, inputs, and expected outputs before any code is written. - 2. 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. - 3. Be ready to add, remove, or rename tests based on user feedback before proceeding. - 4. Once confirmed, write all failing tests first (red), then implement until all pass (green). - 5. Cover all distinct combinations; each test must state its scenario in the docstring. - 6. After all tests pass, update `SPEC.md` with the confirmed test case table (name + intent + input + expected output) to show what is covered. + - 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. + - Must write all failing tests first (red), then implement until all pass (green). + - Must cover all distinct combinations; each test must state its scenario in the docstring. + - Must update `SPEC.md` after all tests pass with the confirmed test case table (name + intent + input + expected output). - Must not add comments outside test methods in `test_*.py` files; use section-header comments (`# --- section ---`) only to separate logical groups of tests. Tooling diff --git a/release_notes_generator/action_inputs.py b/release_notes_generator/action_inputs.py index e346a84c..0e51ad70 100644 --- a/release_notes_generator/action_inputs.py +++ b/release_notes_generator/action_inputs.py @@ -216,8 +216,7 @@ def get_super_chapters() -> list[dict[str, Any]]: if raw_labels is None: logger.warning("Super-chapter '%s' has no 'label' or 'labels' key; skipping", title) continue - labels_input: str | list[str] = [raw_labels] if isinstance(raw_labels, str) else raw_labels - normalized = normalize_labels(labels_input) + normalized = normalize_labels(raw_labels) if not normalized: logger.warning("Super-chapter '%s' labels definition empty after normalization; skipping", title) continue diff --git a/release_notes_generator/model/record/hierarchy_issue_record.py b/release_notes_generator/model/record/hierarchy_issue_record.py index d0c4abf4..47e642f1 100644 --- a/release_notes_generator/model/record/hierarchy_issue_record.py +++ b/release_notes_generator/model/record/hierarchy_issue_record.py @@ -163,7 +163,10 @@ def contains_change_increment(self) -> bool: def get_labels(self) -> list[str]: """Return all labels from this issue, its sub-issues, sub-hierarchy-issues, and attached PRs.""" labels: set[str] = set() - labels.update(label.name for label in self._issue.get_labels()) + if self._labels is not None: + labels.update(self._labels) + else: + labels.update(label.name for label in self._issue.get_labels()) for sub_issue in self._sub_issues.values(): labels.update(sub_issue.labels) diff --git a/tests/unit/conftest.py b/tests/unit/conftest.py index 1ac9d868..172ae070 100644 --- a/tests/unit/conftest.py +++ b/tests/unit/conftest.py @@ -37,9 +37,6 @@ from release_notes_generator.model.chapter import Chapter from typing import Any -from pytest_mock import MockerFixture - -from release_notes_generator.action_inputs import ActionInputs from release_notes_generator.chapters.custom_chapters import CustomChapters from release_notes_generator.model.record.sub_issue_record import SubIssueRecord from release_notes_generator.utils.enums import DuplicityScopeEnum diff --git a/tests/unit/release_notes_generator/test_action_inputs.py b/tests/unit/release_notes_generator/test_action_inputs.py index c97eb6fe..5b06979d 100644 --- a/tests/unit/release_notes_generator/test_action_inputs.py +++ b/tests/unit/release_notes_generator/test_action_inputs.py @@ -249,6 +249,16 @@ def test_get_super_chapters_list_labels_preserved(mocker): assert result == [{"title": "Module A", "labels": ["mod-a", "mod-b"]}] +def test_get_super_chapters_comma_separated_labels_string(mocker): + """Comma-separated string labels are split into individual label tokens.""" + mocker.patch( + "release_notes_generator.action_inputs.get_action_input", + return_value='[{"title": "Module A", "labels": "atum-agent, atum-agent-spark"}]', + ) + result = ActionInputs.get_super_chapters() + assert result == [{"title": "Module A", "labels": ["atum-agent", "atum-agent-spark"]}] + + def test_get_super_chapters_non_dict_entry_skipped(mocker, caplog): caplog.set_level("WARNING") mocker.patch( From f72bcc598c31eed370e69a0e10841739caf0a80a Mon Sep 17 00:00:00 2001 From: miroslavpojer Date: Mon, 6 Apr 2026 19:24:21 +0200 Subject: [PATCH 17/20] Fix review comments. --- tests/unit/conftest.py | 16 +++- .../chapters/test_custom_chapters.py | 6 -- .../model/test_hierarchy_issue_record.py | 78 +++++++------------ 3 files changed, 39 insertions(+), 61 deletions(-) diff --git a/tests/unit/conftest.py b/tests/unit/conftest.py index 172ae070..8ec9b3d0 100644 --- a/tests/unit/conftest.py +++ b/tests/unit/conftest.py @@ -1295,9 +1295,13 @@ def make_minimal_pr(mocker: MockerFixture, number: int) -> PullRequest: return pr -def make_closed_sub_issue_record_with_pr(mocker: MockerFixture, number: int) -> SubIssueRecord: +def make_closed_sub_issue_record_with_pr( + mocker: MockerFixture, number: int, issue_labels: list[str] | None = None +) -> SubIssueRecord: """Return a closed SubIssueRecord with one PR (no commits).""" - sub_record = SubIssueRecord(make_minimal_issue(mocker, IssueRecord.ISSUE_STATE_CLOSED, number)) + sub_record = SubIssueRecord( + make_minimal_issue(mocker, IssueRecord.ISSUE_STATE_CLOSED, number), issue_labels=issue_labels + ) sub_record.register_pull_request(make_minimal_pr(mocker, number=number + 1000)) return sub_record @@ -1324,9 +1328,13 @@ def make_open_sub_hierarchy_record_with_pr(mocker: MockerFixture, number: int) - return rec -def make_closed_sub_hierarchy_record_with_pr(mocker: MockerFixture, number: int) -> HierarchyIssueRecord: +def make_closed_sub_hierarchy_record_with_pr( + mocker: MockerFixture, number: int, issue_labels: list[str] | None = None +) -> HierarchyIssueRecord: """Return a closed HierarchyIssueRecord with one PR (no commits).""" - rec = HierarchyIssueRecord(make_minimal_issue(mocker, IssueRecord.ISSUE_STATE_CLOSED, number)) + rec = HierarchyIssueRecord( + make_minimal_issue(mocker, IssueRecord.ISSUE_STATE_CLOSED, number), issue_labels=issue_labels + ) rec.register_pull_request(make_minimal_pr(mocker, number=number + 1000)) return rec diff --git a/tests/unit/release_notes_generator/chapters/test_custom_chapters.py b/tests/unit/release_notes_generator/chapters/test_custom_chapters.py index d9fcf188..e7c8c0e6 100644 --- a/tests/unit/release_notes_generator/chapters/test_custom_chapters.py +++ b/tests/unit/release_notes_generator/chapters/test_custom_chapters.py @@ -1681,10 +1681,6 @@ def test_sc_combo_c3b_three_level_hierarchy_split(mocker, hierarchy_with_sub_iss sub_issue_stubs=[SI("org/repo#2", ["security", "scope:security", "enhancement"])], ) # Override to_chapter_row and helper methods so Feature renders correctly - epic._sub_stubs = [] - - original_to_row = epic.to_chapter_row.__func__ if hasattr(epic.to_chapter_row, "__func__") else None - def epic_to_chapter_row(add_into_chapters=True, label_filter=None, exclude_labels=None): row = f"org/repo#1 row" sub_row = feature.to_chapter_row( @@ -1694,7 +1690,6 @@ def epic_to_chapter_row(add_into_chapters=True, label_filter=None, exclude_label row += f"\n {sub_row}" return row - import types epic.to_chapter_row = epic_to_chapter_row def epic_has_matching_labels(label_filter): @@ -1711,7 +1706,6 @@ def epic_get_labels(): epic.has_matching_labels = epic_has_matching_labels epic.has_unmatched_descendants = epic_has_unmatched_descendants epic.get_labels = epic_get_labels - epic._labels = ["epic", "enhancement"] + feature.get_labels() cc = make_super_chapters_cc( mocker, diff --git a/tests/unit/release_notes_generator/model/test_hierarchy_issue_record.py b/tests/unit/release_notes_generator/model/test_hierarchy_issue_record.py index ff201be1..61efff4c 100644 --- a/tests/unit/release_notes_generator/model/test_hierarchy_issue_record.py +++ b/tests/unit/release_notes_generator/model/test_hierarchy_issue_record.py @@ -560,10 +560,8 @@ def test_to_chapter_row_label_filter_includes_matching_sub_issues(mocker, patch_ parent = make_minimal_issue(mocker, IssueRecord.ISSUE_STATE_CLOSED, number=1) record = HierarchyIssueRecord(parent) - fe_sub = make_closed_sub_issue_record_with_pr(mocker, number=2) - fe_sub._labels = ["enhancement", "frontend"] - be_sub = make_closed_sub_issue_record_with_pr(mocker, number=3) - be_sub._labels = ["enhancement", "backend"] + fe_sub = make_closed_sub_issue_record_with_pr(mocker, number=2, issue_labels=["enhancement", "frontend"]) + be_sub = make_closed_sub_issue_record_with_pr(mocker, number=3, issue_labels=["enhancement", "backend"]) record.sub_issues["2"] = fe_sub record.sub_issues["3"] = be_sub @@ -578,10 +576,8 @@ def test_to_chapter_row_label_filter_includes_matching_sub_hierarchy(mocker, pat parent = make_minimal_issue(mocker, IssueRecord.ISSUE_STATE_CLOSED, number=1) record = HierarchyIssueRecord(parent) - fe_child = make_closed_sub_hierarchy_record_with_pr(mocker, number=10) - fe_child._labels = ["frontend"] - be_child = make_closed_sub_hierarchy_record_with_pr(mocker, number=20) - be_child._labels = ["backend"] + fe_child = make_closed_sub_hierarchy_record_with_pr(mocker, number=10, issue_labels=["frontend"]) + be_child = make_closed_sub_hierarchy_record_with_pr(mocker, number=20, issue_labels=["backend"]) record.sub_hierarchy_issues["10"] = fe_child record.sub_hierarchy_issues["20"] = be_child @@ -596,10 +592,8 @@ def test_to_chapter_row_no_label_filter_renders_all(mocker, patch_hierarchy_acti parent = make_minimal_issue(mocker, IssueRecord.ISSUE_STATE_CLOSED, number=1) record = HierarchyIssueRecord(parent) - fe_sub = make_closed_sub_issue_record_with_pr(mocker, number=2) - fe_sub._labels = ["frontend"] - be_sub = make_closed_sub_issue_record_with_pr(mocker, number=3) - be_sub._labels = ["backend"] + fe_sub = make_closed_sub_issue_record_with_pr(mocker, number=2, issue_labels=["frontend"]) + be_sub = make_closed_sub_issue_record_with_pr(mocker, number=3, issue_labels=["backend"]) record.sub_issues["2"] = fe_sub record.sub_issues["3"] = be_sub @@ -658,10 +652,8 @@ def test_to_chapter_row_exclude_labels_hides_matching_sub_issues(mocker, patch_h parent = make_minimal_issue(mocker, IssueRecord.ISSUE_STATE_CLOSED, number=1) record = HierarchyIssueRecord(parent) - sec_sub = make_closed_sub_issue_record_with_pr(mocker, number=2) - sec_sub._labels = ["enhancement", "scope:security"] - plain_sub = make_closed_sub_issue_record_with_pr(mocker, number=3) - plain_sub._labels = ["enhancement"] + sec_sub = make_closed_sub_issue_record_with_pr(mocker, number=2, issue_labels=["enhancement", "scope:security"]) + plain_sub = make_closed_sub_issue_record_with_pr(mocker, number=3, issue_labels=["enhancement"]) record.sub_issues["2"] = sec_sub record.sub_issues["3"] = plain_sub @@ -681,10 +673,8 @@ def test_to_chapter_row_exclude_labels_hides_matching_sub_hierarchy(mocker, patc parent = make_minimal_issue(mocker, IssueRecord.ISSUE_STATE_CLOSED, number=1) record = HierarchyIssueRecord(parent) - sec_child = make_closed_sub_hierarchy_record_with_pr(mocker, number=10) - sec_child._labels = ["scope:security"] - plain_child = make_closed_sub_hierarchy_record_with_pr(mocker, number=20) - plain_child._labels = ["other"] + sec_child = make_closed_sub_hierarchy_record_with_pr(mocker, number=10, issue_labels=["scope:security"]) + plain_child = make_closed_sub_hierarchy_record_with_pr(mocker, number=20, issue_labels=["other"]) record.sub_hierarchy_issues["10"] = sec_child record.sub_hierarchy_issues["20"] = plain_child @@ -699,10 +689,8 @@ def test_to_chapter_row_exclude_labels_none_renders_all(mocker, patch_hierarchy_ parent = make_minimal_issue(mocker, IssueRecord.ISSUE_STATE_CLOSED, number=1) record = HierarchyIssueRecord(parent) - s1 = make_closed_sub_issue_record_with_pr(mocker, number=2) - s1._labels = ["scope:security"] - s2 = make_closed_sub_issue_record_with_pr(mocker, number=3) - s2._labels = ["other"] + s1 = make_closed_sub_issue_record_with_pr(mocker, number=2, issue_labels=["scope:security"]) + s2 = make_closed_sub_issue_record_with_pr(mocker, number=3, issue_labels=["other"]) record.sub_issues["2"] = s1 record.sub_issues["3"] = s2 @@ -727,13 +715,10 @@ def test_to_chapter_row_exclude_labels_keeps_sub_hierarchy_with_mixed_descendant """ epic = HierarchyIssueRecord(make_minimal_issue(mocker, IssueRecord.ISSUE_STATE_CLOSED, number=1)) - feature = HierarchyIssueRecord(make_minimal_issue(mocker, IssueRecord.ISSUE_STATE_CLOSED, number=10)) - feature._labels = ["security"] + feature = HierarchyIssueRecord(make_minimal_issue(mocker, IssueRecord.ISSUE_STATE_CLOSED, number=10), issue_labels=["security"]) - task1 = make_closed_sub_issue_record_with_pr(mocker, number=20) - task1._labels = ["enhancement"] - task2 = make_closed_sub_issue_record_with_pr(mocker, number=21) - task2._labels = ["enhancement", "scope:security"] + task1 = make_closed_sub_issue_record_with_pr(mocker, number=20, issue_labels=["enhancement"]) + task2 = make_closed_sub_issue_record_with_pr(mocker, number=21, issue_labels=["enhancement", "scope:security"]) feature.sub_issues["20"] = task1 feature.sub_issues["21"] = task2 @@ -752,11 +737,9 @@ def test_to_chapter_row_exclude_labels_hides_sub_hierarchy_when_all_descendants_ """Sub-hierarchy issue is hidden when ALL its descendants match exclude_labels.""" epic = HierarchyIssueRecord(make_minimal_issue(mocker, IssueRecord.ISSUE_STATE_CLOSED, number=1)) - feature = HierarchyIssueRecord(make_minimal_issue(mocker, IssueRecord.ISSUE_STATE_CLOSED, number=10)) - feature._labels = ["security"] + feature = HierarchyIssueRecord(make_minimal_issue(mocker, IssueRecord.ISSUE_STATE_CLOSED, number=10), issue_labels=["security"]) - task2 = make_closed_sub_issue_record_with_pr(mocker, number=21) - task2._labels = ["enhancement", "scope:security"] + task2 = make_closed_sub_issue_record_with_pr(mocker, number=21, issue_labels=["enhancement", "scope:security"]) feature.sub_issues["21"] = task2 epic.sub_hierarchy_issues["10"] = feature @@ -854,30 +837,23 @@ def test_five_level_hierarchy_label_filter_and_exclude_labels(mocker, patch_hier and omits Task #10 """ # Build bottom-up: Task leaves first - task_sc = make_closed_sub_issue_record_with_pr(mocker, number=10) - task_sc._labels = ["scope:security"] - task_plain = make_closed_sub_issue_record_with_pr(mocker, number=11) - task_plain._labels = ["enhancement"] - - story = HierarchyIssueRecord(make_minimal_issue(mocker, IssueRecord.ISSUE_STATE_CLOSED, number=4)) - story._labels = ["story"] - story._level = 3 + task_sc = make_closed_sub_issue_record_with_pr(mocker, number=10, issue_labels=["scope:security"]) + task_plain = make_closed_sub_issue_record_with_pr(mocker, number=11, issue_labels=["enhancement"]) + + story = HierarchyIssueRecord(make_minimal_issue(mocker, IssueRecord.ISSUE_STATE_CLOSED, number=4), issue_labels=["story"]) + story.level = 3 story.sub_issues["10"] = task_sc story.sub_issues["11"] = task_plain - feature = HierarchyIssueRecord(make_minimal_issue(mocker, IssueRecord.ISSUE_STATE_CLOSED, number=3)) - feature._labels = ["feature"] - feature._level = 2 + feature = HierarchyIssueRecord(make_minimal_issue(mocker, IssueRecord.ISSUE_STATE_CLOSED, number=3), issue_labels=["feature"]) + feature.level = 2 feature.sub_hierarchy_issues["4"] = story - theme = HierarchyIssueRecord(make_minimal_issue(mocker, IssueRecord.ISSUE_STATE_CLOSED, number=2)) - theme._labels = ["theme"] - theme._level = 1 + theme = HierarchyIssueRecord(make_minimal_issue(mocker, IssueRecord.ISSUE_STATE_CLOSED, number=2), issue_labels=["theme"]) + theme.level = 1 theme.sub_hierarchy_issues["3"] = feature - epic = HierarchyIssueRecord(make_minimal_issue(mocker, IssueRecord.ISSUE_STATE_CLOSED, number=1)) - epic._labels = ["epic"] - epic._level = 0 + epic = HierarchyIssueRecord(make_minimal_issue(mocker, IssueRecord.ISSUE_STATE_CLOSED, number=1), issue_labels=["epic"]) epic.sub_hierarchy_issues["2"] = theme # --- get_labels aggregates across all 5 levels --- From 2da077cd266eddeff756b2751254c33efccb0939 Mon Sep 17 00:00:00 2001 From: miroslavpojer Date: Mon, 6 Apr 2026 19:45:35 +0200 Subject: [PATCH 18/20] Fix review comments. --- .../chapters/custom_chapters.py | 10 ++- .../chapters/test_custom_chapters.py | 64 +++++++++++++++++-- 2 files changed, 67 insertions(+), 7 deletions(-) diff --git a/release_notes_generator/chapters/custom_chapters.py b/release_notes_generator/chapters/custom_chapters.py index 3b67670c..63b52eef 100644 --- a/release_notes_generator/chapters/custom_chapters.py +++ b/release_notes_generator/chapters/custom_chapters.py @@ -212,6 +212,7 @@ def _render_chapter_for_ids( matching_ids: set[str], super_labels: list[str] | None = None, exclude_labels: list[str] | None = None, + print_empty_chapters: bool | None = None, ) -> str: """Render a single chapter filtered to matching_ids, delegating to Chapter.to_string. @@ -222,7 +223,10 @@ def _render_chapter_for_ids( filtered to those whose labels intersect *super_labels*. exclude_labels: When set, HierarchyIssueRecords are re-rendered with sub-issues whose labels intersect *exclude_labels* removed (for Uncategorized fallback). + print_empty_chapters: Override for the empty-chapter setting; defaults to + ``self.print_empty_chapters`` when ``None``. """ + effective_print_empty = self.print_empty_chapters if print_empty_chapters is None else print_empty_chapters original_rows = chapter.rows filtered_rows: dict[int | str, str] = {} for rid, row in original_rows.items(): @@ -239,7 +243,7 @@ def _render_chapter_for_ids( filtered_rows[rid] = row chapter.rows = filtered_rows try: - return chapter.to_string(sort_ascending=self.sort_ascending, print_empty_chapters=self.print_empty_chapters) + return chapter.to_string(sort_ascending=self.sort_ascending, print_empty_chapters=effective_print_empty) finally: chapter.rows = original_rows @@ -288,7 +292,9 @@ def _render_uncategorized_block(self, all_uncat_ids: set[str], all_super_labels_ for chapter in self._sorted_chapters(): if chapter.hidden: continue - ch_str = self._render_chapter_for_ids(chapter, all_uncat_ids, exclude_labels=all_super_labels_list) + ch_str = self._render_chapter_for_ids( + chapter, all_uncat_ids, exclude_labels=all_super_labels_list, print_empty_chapters=False + ) if ch_str: uc_block += ch_str + "\n\n" if uc_block.strip(): diff --git a/tests/unit/release_notes_generator/chapters/test_custom_chapters.py b/tests/unit/release_notes_generator/chapters/test_custom_chapters.py index e7c8c0e6..f2bc4fb1 100644 --- a/tests/unit/release_notes_generator/chapters/test_custom_chapters.py +++ b/tests/unit/release_notes_generator/chapters/test_custom_chapters.py @@ -1873,7 +1873,11 @@ def test_from_yaml_array_repeated_chapter_with_hidden_logs_info(caplog): def test_super_chapters_hidden_chapter_skipped_in_sc_and_uncat_loops(mocker, record_stub): - """Hidden chapters are skipped during both SC-block and Uncategorized-block rendering.""" + """Hidden chapters are skipped during both SC-block and Uncategorized-block rendering. + + When the only unclaimed record lives in a hidden chapter, visible chapters produce + no matching rows for Uncategorized, so ## Uncategorized must not be emitted. + """ cc = make_super_chapters_cc( mocker, [ @@ -1884,7 +1888,7 @@ def test_super_chapters_hidden_chapter_skipped_in_sc_and_uncat_loops(mocker, rec print_empty=True, ) r_sc = record_stub("org/repo#1", ["sc-label"]) - r_plain = record_stub("org/repo#2", ["other-label"]) # assigned to hidden chapter + r_plain = record_stub("org/repo#2", ["other-label"]) # assigned only to hidden chapter cc.populate({"org/repo#1": r_sc, "org/repo#2": r_plain}) output = cc.to_string() @@ -1892,9 +1896,9 @@ def test_super_chapters_hidden_chapter_skipped_in_sc_and_uncat_loops(mocker, rec # SC section renders with visible chapter (hidden chapter is skipped in SC loop) assert "## Security" in output assert "org/repo#1 row" in output - # Uncategorized section is entered (r_plain is unclaimed) but hidden chapter is skipped, - # so r_plain does NOT appear in the rendered output - assert "Uncategorized" in output + # r_plain is unclaimed but lives only in a hidden chapter; the visible chapter has no + # matching uncategorized rows → uc_block is empty → ## Uncategorized must NOT appear + assert "Uncategorized" not in output assert "org/repo#2 row" not in output @@ -2017,3 +2021,53 @@ def test_collect_uncategorized_ids_no_rows_produces_no_uncat_section(mocker): ) output = cc.to_string() assert "Uncategorized" not in output + + +def test_uncategorized_no_empty_sub_chapters_when_print_empty_true(mocker, record_stub): + """print-empty-chapters=True must not produce empty ### sub-chapters inside ## Uncategorized.""" + cc = make_super_chapters_cc( + mocker, + [ + {"title": "Bugs", "label": "bug"}, + {"title": "Features", "label": "feature"}, + ], + [{"title": "Security", "labels": ["scope:security"]}], + print_empty=True, + ) + # Only a 'bug' record with no SC label → goes to Uncategorized under Bugs only + r1 = record_stub("org/repo#1", ["bug"]) + cc.populate({"org/repo#1": r1}) + output = cc.to_string() + + assert "## Uncategorized" in output + uncat_section = output.split("## Uncategorized")[1] + assert "org/repo#1 row" in uncat_section + # Features chapter has no matching records → must NOT appear inside Uncategorized + assert "Features" not in uncat_section, ( + "Empty sub-chapter 'Features' must not appear in Uncategorized regardless of print-empty-chapters setting" + ) + + +def test_uncategorized_not_emitted_when_only_visible_unclaimed_records_comes_from_empty_chapters( + mocker, record_stub +): + """## Uncategorized is not emitted when visible chapters have no matching unclaimed rows. + + Distinct from the hidden-chapter case: here a visible chapter has a record but that + record is SC-claimed, so the filtered rows for Uncategorized are empty for every + visible chapter → no ## Uncategorized should appear. + """ + cc = make_super_chapters_cc( + mocker, + [{"title": "Features", "label": "feature"}], + [{"title": "Security", "labels": ["feature"]}], + print_empty=True, + ) + # r1 matches feature AND the SC label → it is claimed; Uncategorized would be empty + r1 = record_stub("org/repo#1", ["feature"]) + cc.populate({"org/repo#1": r1}) + output = cc.to_string() + + assert "Uncategorized" not in output, ( + "## Uncategorized must not appear when all records are SC-claimed" + ) From 8382e556f2986ed67499f16457701ef306f0b527 Mon Sep 17 00:00:00 2001 From: miroslavpojer Date: Mon, 6 Apr 2026 20:08:08 +0200 Subject: [PATCH 19/20] Fixed review comments. --- .../chapters/custom_chapters.py | 2 +- .../model/record/hierarchy_issue_record.py | 13 +--- .../chapters/test_custom_chapters.py | 77 +++++-------------- .../model/test_hierarchy_issue_record.py | 13 ++++ 4 files changed, 39 insertions(+), 66 deletions(-) diff --git a/release_notes_generator/chapters/custom_chapters.py b/release_notes_generator/chapters/custom_chapters.py index 63b52eef..9c49af1f 100644 --- a/release_notes_generator/chapters/custom_chapters.py +++ b/release_notes_generator/chapters/custom_chapters.py @@ -232,7 +232,7 @@ def _render_chapter_for_ids( for rid, row in original_rows.items(): if str(rid) not in matching_ids and rid not in matching_ids: continue - record = self._records.get(str(rid)) or self._records.get(rid) # type: ignore[arg-type] + record = self._records.get(str(rid)) if super_labels and isinstance(record, HierarchyIssueRecord): if not record.has_matching_labels(super_labels): continue diff --git a/release_notes_generator/model/record/hierarchy_issue_record.py b/release_notes_generator/model/record/hierarchy_issue_record.py index 47e642f1..8236d77d 100644 --- a/release_notes_generator/model/record/hierarchy_issue_record.py +++ b/release_notes_generator/model/record/hierarchy_issue_record.py @@ -182,21 +182,16 @@ def get_labels(self) -> list[str]: def has_matching_labels(self, label_filter: list[str]) -> bool: """Check if this hierarchy issue or any descendant has labels matching the filter. + Uses the same aggregated label set as get_labels() so that PR labels attached to + this record or its descendants are considered when matching super-chapter filters. + Parameters: label_filter: Labels to check against. Returns: True if this record or any descendant has at least one matching label. """ - if any(lbl in label_filter for lbl in self.labels): - return True - for sub_issue in self._sub_issues.values(): - if any(lbl in label_filter for lbl in sub_issue.labels): - return True - for sub_hierarchy_issue in self._sub_hierarchy_issues.values(): - if sub_hierarchy_issue.has_matching_labels(label_filter): - return True - return False + return any(lbl in label_filter for lbl in self.get_labels()) def has_unmatched_descendants(self, all_super_labels: list[str]) -> bool: """Check if any descendant does NOT match any label in the combined super-chapter label set. diff --git a/tests/unit/release_notes_generator/chapters/test_custom_chapters.py b/tests/unit/release_notes_generator/chapters/test_custom_chapters.py index f2bc4fb1..44e4d337 100644 --- a/tests/unit/release_notes_generator/chapters/test_custom_chapters.py +++ b/tests/unit/release_notes_generator/chapters/test_custom_chapters.py @@ -15,6 +15,8 @@ # import pytest +from github.Issue import Issue + from release_notes_generator.chapters.custom_chapters import CustomChapters from release_notes_generator.utils.utils import normalize_labels from release_notes_generator.model.record.issue_record import IssueRecord @@ -827,38 +829,22 @@ def test_sorted_chapters_hidden_with_order(): @pytest.fixture -def hierarchy_record_stub(): +def hierarchy_record_stub(mocker): """Create a minimal HierarchyIssueRecord-like stub for catch-open-hierarchy tests.""" class HierarchyRecordStub(HierarchyIssueRecord): """Stub that avoids calling real GitHub Issue methods.""" - # noinspection PyMissingConstructor def __init__(self, rid, labels, state, contains_change): - # Bypass real __init__ to avoid GitHub API objects - Record.__init__(self, labels=labels, skip=False) + mock_issue = mocker.MagicMock(spec=Issue) + mock_issue.state = state + mock_issue.number = rid + mock_issue.type = None + mock_issue.user = None + mock_issue.assignees = [] + super().__init__(mock_issue, issue_labels=labels) self._rid = rid - self._state = state self._contains = contains_change - self._level = 0 - self._sub_issues = {} - self._sub_hierarchy_issues = {} - self._issue = None - self._pull_requests = {} - self._commits = {} - self._issue_type = None - - @property - def record_id(self): - return self._rid - - @property - def is_closed(self): - return self._state == "closed" - - @property - def is_open(self): - return self._state == "open" @property def author(self): @@ -1395,7 +1381,7 @@ def test_super_chapters_coh_record_visible_in_fallback(mocker, hierarchy_record_ @pytest.fixture -def hierarchy_with_sub_issues_stub(): +def hierarchy_with_sub_issues_stub(mocker): """Create a HierarchyIssueRecord-like stub that renders sub-issues filtered by label_filter.""" class SubIssueStub: @@ -1410,33 +1396,17 @@ def labels(self): class HierarchyWithSubsStub(HierarchyIssueRecord): """Stub with sub-issue-aware to_chapter_row that respects label_filter.""" - # noinspection PyMissingConstructor def __init__(self, rid, labels, sub_issue_stubs, state="closed"): - Record.__init__(self, labels=labels, skip=False) + mock_issue = mocker.MagicMock(spec=Issue) + mock_issue.state = state + mock_issue.number = rid + mock_issue.type = None + mock_issue.user = None + mock_issue.assignees = [] + super().__init__(mock_issue, issue_labels=labels) self._rid = rid - self._state = state - self._contains = True - self._level = 0 - self._sub_issues = {} - self._sub_hierarchy_issues = {} - self._issue = None - self._pull_requests = {} - self._commits = {} - self._issue_type = None self._sub_stubs: list[SubIssueStub] = sub_issue_stubs - @property - def record_id(self): - return self._rid - - @property - def is_closed(self): - return self._state == "closed" - - @property - def is_open(self): - return self._state == "open" - @property def author(self): return "author" @@ -1460,21 +1430,16 @@ def to_chapter_row(self, add_into_chapters=True, label_filter=None, exclude_labe return row def contains_change_increment(self): - return self._contains + return True def get_labels(self): - all_labels = list(self._labels or []) + all_labels = list(self.labels) for stub in self._sub_stubs: all_labels.extend(stub.labels) return all_labels def has_matching_labels(self, label_filter): - if any(lbl in label_filter for lbl in (self._labels or [])): - return True - for stub in self._sub_stubs: - if any(lbl in label_filter for lbl in stub.labels): - return True - return False + return any(lbl in label_filter for lbl in self.get_labels()) def has_unmatched_descendants(self, all_super_labels): if not self._sub_stubs: diff --git a/tests/unit/release_notes_generator/model/test_hierarchy_issue_record.py b/tests/unit/release_notes_generator/model/test_hierarchy_issue_record.py index 61efff4c..5f1c7f26 100644 --- a/tests/unit/release_notes_generator/model/test_hierarchy_issue_record.py +++ b/tests/unit/release_notes_generator/model/test_hierarchy_issue_record.py @@ -552,6 +552,19 @@ def test_has_matching_labels_no_match_returns_false(make_hierarchy_issue): assert record.has_matching_labels(["frontend"]) is False +def test_has_matching_labels_pr_label_matches(mocker, make_hierarchy_issue): + """A label on an attached PR satisfies the filter (aligns with get_labels behaviour).""" + issue = make_hierarchy_issue(100, IssueRecord.ISSUE_STATE_CLOSED) + record = HierarchyIssueRecord(issue, issue_labels=["epic"]) + + pr = make_minimal_pr(mocker, number=1100) + pr.get_labels.return_value = [type("L", (), {"name": "frontend"})()] + record.register_pull_request(pr) + + assert record.has_matching_labels(["frontend"]) is True + assert record.has_matching_labels(["backend"]) is False + + # --- to_chapter_row with label_filter --- From 85774a5351e4ca68443ee3211a3b2f03a4a45414 Mon Sep 17 00:00:00 2001 From: miroslavpojer Date: Mon, 6 Apr 2026 20:24:09 +0200 Subject: [PATCH 20/20] Fixed review comment. --- release_notes_generator/model/record/hierarchy_issue_record.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/release_notes_generator/model/record/hierarchy_issue_record.py b/release_notes_generator/model/record/hierarchy_issue_record.py index 8236d77d..1201711e 100644 --- a/release_notes_generator/model/record/hierarchy_issue_record.py +++ b/release_notes_generator/model/record/hierarchy_issue_record.py @@ -321,7 +321,8 @@ def _append_sub_issue_rows( open_icon_prefix = "" if self.is_closed and sub_issue.is_open: # Highlight open children under a closed parent to signal incomplete work - open_icon_prefix = ActionInputs.get_open_hierarchy_sub_issue_icon() + " " + icon = ActionInputs.get_open_hierarchy_sub_issue_icon() + open_icon_prefix = (icon + " ") if icon else "" sub_issue_block = "- " + open_icon_prefix + sub_issue.to_chapter_row() ind_child_block = "\n".join(f"{sub_indent}{line}" if line else "" for line in sub_issue_block.splitlines()) row = f"{row}\n{ind_child_block}"