#195 - Update rules for service chapter "Merged PRs Linked to 'Not Cl…

[miroslavpojer]

Release Notes:

• Improved rules for deciding when hierarchy issue can be included into service chapter "Merged PRs Linked to 'Not Closed' Issue". Hierarchy issue can be open when cotains merged PRs.

Summary by CodeRabbit

• Bug Fixes

  • Improved handling of open vs. closed issue statuses so merged PRs linked to open issues are flagged appropriately and hierarchy duplicates are avoided.
  • Consolidated messaging: when all merged PRs are linked to closed issues, release notes show a single clear statement.

• Chores

  • Added logging to trace skipped hierarchical entries and runtime decisions.

• Tests

  • Updated tests to expect the consolidated merged-PRs message and added debug traces in select tests.

Fixes #195

[coderabbitai]

Walkthrough

Reworks service_chapters.populate else-branch to handle open vs closed IssueRecord/SubIssueRecord/HierarchyIssueRecord, add module logger, and log when open issues with linked PRs are collected. Tests updated to collapse per-PR "Merged PRs Linked to 'Not Closed' Issue" sections into a single declarative line.

Changes

Cohort / File(s)	Summary
Service chapter logic updates release_notes_generator/chapters/service_chapters.py	Reworks populate() else-branch: adds pr_count, is_issue_like, is_hierarchy checks; skips open HierarchyIssueRecord present in chapters (logged); if open issue/sub-issue has PRs, adds to MERGED_PRS_LINKED_TO_NOT_CLOSED_ISSUES and marks used (debug log); closed records added to OTHERS_NO_TOPIC if not recorded; adds module-level logger and imports HierarchyIssueRecord, SubIssueRecord.
Tests alignment tests/release_notes/builder/test_release_notes_builder.py	Replaces three per-PR "Merged PRs Linked to 'Not Closed' Issue ⚠️" sections with a single statement: "All merged PRs are linked to Closed issues." No other structural test changes.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor Builder
  participant ServiceChapters as ServiceChapters.populate
  participant Record as Issue/SubIssue/HierarchyRecord
  participant Chapters as Chapters Index
  participant Buckets as Buckets (MERGED_PRS..., OTHERS_NO_TOPIC)
  note over ServiceChapters: Else-branch control flow (modified)

  Builder->>ServiceChapters: populate(records)
  loop For each record
    ServiceChapters->>Record: inspect type/status/PR count
    alt Record is open
      alt HierarchyIssueRecord and present in Chapters
        ServiceChapters->>ServiceChapters: log skip (hierarchy open)
      else Issue/SubIssue with PRs (>0)
        ServiceChapters->>Buckets: add to MERGED_PRS_LINKED_TO_NOT_CLOSED_ISSUES
        ServiceChapters->>ServiceChapters: debug log (open issue with PRs)
      else Open with no PRs
        ServiceChapters->>ServiceChapters: no-op
      end
    else Record is closed
      ServiceChapters->>Buckets: add to OTHERS_NO_TOPIC if not recorded
    end
  end
  ServiceChapters-->>Builder: done

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

• #152 - Refactor Record call #154 — Also modifies release_notes_generator/chapters/service_chapters.py, likely touching related type-based population logic.

Suggested labels

enhancement

Suggested reviewers

• Zejnilovic
• OlivieFranklova

Poem

A rabbit hops through issues neat,
Skips tall hierarchies with nimble feet.
PRs linked to open friends get a gentle log,
Closed ones shelved inside the bog.
I thump a note — release notes tidy and sweet. 🐇✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Description Check	⚠️ Warning	The description does not follow the required template because it lacks the mandatory Checklist and Additional Context sections and does not provide a concise PR summary under the prescribed heading.	Please restructure the description to match the repository template by adding a brief PR Summary, the completed Checklist items, and an Additional Context section above the “Fixes #195” footer.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title Check	✅ Passed	The title clearly summarizes the primary change by stating the update to the service chapter rules for “Merged PRs Linked to 'Not Closed' Issue” and directly relates to the linked issue, making it easy to understand the main purpose.
Linked Issues Check	✅ Passed	The changes introduce pr_count and is_hierarchy flags to the service chapter logic and adjust tests so that open hierarchical issues containing merged PRs are correctly included in the “Merged PRs Linked to 'Not Closed' Issue” chapter as specified in issue #195, fulfilling the updated rules requirement.
Out of Scope Changes Check	✅ Passed	All modifications are focused on updating the service chapter behavior and adapting the corresponding builder tests to reflect the new rules for “Merged PRs Linked to 'Not Closed' Issue,” and there are no unrelated changes.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch feature/195-Update-rules-for-service-chapter-Merged-PRs-Linked-to-Not-Closed-Issue

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

tests/release_notes/builder/test_release_notes_builder.py (1)

1561-1563: Remove leftover debug prints from tests.

The added print("XXX") wrappers only add noise to test output and look like forgotten debugging aids. Please drop them.

-    print("XXX")
-    print(actual_release_notes)
-    print("XXX")

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 80b27be and e5adbe1.

📒 Files selected for processing (2)

• release_notes_generator/chapters/service_chapters.py (4 hunks)
• tests/release_notes/builder/test_release_notes_builder.py (4 hunks)

🧰 Additional context used

🧬 Code graph analysis (1)

release_notes_generator/chapters/service_chapters.py (4)

release_notes_generator/model/record/hierarchy_issue_record.py (3)

• HierarchyIssueRecord (16-196)
• pull_requests_count (84-99)
• to_chapter_row (117-182)

release_notes_generator/model/record/issue_record.py (5)

• IssueRecord (17-268)
• pull_requests_count (248-254)
• is_open (52-53)
• record_id (44-45)
• to_chapter_row (130-156)

release_notes_generator/model/record/record.py (5)

• Record (32-258)
• is_open (112-117)
• is_present_in_chapters (49-55)
• record_id (94-99)
• to_chapter_row (154-163)

release_notes_generator/model/record/sub_issue_record.py (1)

• SubIssueRecord (15-26)

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (1)

release_notes_generator/chapters/service_chapters.py (1)

129-145: Guard or document the unchecked IssueRecord cast
In release_notes_generator/chapters/service_chapters.py line 129, wrap the call to pull_requests_count() in an isinstance(record, IssueRecord) check—or add a comment explaining why only IssueRecord subclasses ever reach this branch—to prevent a future AttributeError if new Record types are introduced.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e5adbe1 and f07c490.

📒 Files selected for processing (2)

• release_notes_generator/chapters/service_chapters.py (3 hunks)
• tests/release_notes/builder/test_release_notes_builder.py (3 hunks)

🧰 Additional context used

🧬 Code graph analysis (1)

release_notes_generator/chapters/service_chapters.py (4)

release_notes_generator/model/record/hierarchy_issue_record.py (3)

• HierarchyIssueRecord (16-196)
• pull_requests_count (84-99)
• to_chapter_row (117-182)

release_notes_generator/model/record/issue_record.py (5)

• IssueRecord (17-268)
• pull_requests_count (248-254)
• is_open (52-53)
• record_id (44-45)
• to_chapter_row (130-156)

release_notes_generator/model/record/record.py (5)

• Record (32-258)
• is_open (112-117)
• is_present_in_chapters (49-55)
• record_id (94-99)
• to_chapter_row (154-163)

release_notes_generator/model/record/sub_issue_record.py (1)

• SubIssueRecord (15-26)

🔇 Additional comments (4)

tests/release_notes/builder/test_release_notes_builder.py (1)

272-272: LGTM! Test expectations correctly updated.

The test updates properly reflect the new behavior where hierarchical issues with labels/types are placed in dedicated chapters (e.g., "Epics"), leaving the service chapter with its empty-state message rather than duplicating entries.

Also applies to: 336-336, 403-403

release_notes_generator/chapters/service_chapters.py (3)

21-21: LGTM! Proper logging and import additions.

The logging infrastructure and new record type imports correctly support the enhanced else-branch logic that handles hierarchical and sub-issue records.

Also applies to: 28-28, 32-32, 44-44

---

133-145: LGTM! Correctly implements the PR objectives.

The logic properly handles the new requirements:

1. Line 134-135: Skips open HierarchyIssueRecords already present in other chapters (e.g., "Epics"), preventing duplication
2. Lines 136-142: Adds open issues/sub-issues with linked PRs to the MERGED_PRS_LINKED_TO_NOT_CLOSED_ISSUES chapter, fulfilling the PR objective that "hierarchical issues which are open but contain merged PRs are not treated as errors"
3. Lines 143-145: Preserves original behavior for open issues without PRs (no-op)

The debug logging at lines 135 and 141 aids troubleshooting.

---

146-149: LGTM! Appropriate fallback for edge cases.

The else clause (lines 146-149) provides a safe fallback for closed records not handled by earlier branches, adding them to OTHERS_NO_TOPIC while preventing duplicates by checking used_record_numbers.