Skip to content

Allow ignoring PRs with specific labels from review#245

Merged
dgershman merged 1 commit intomainfrom
feature/crow-244-ignore-labels-review
May 7, 2026
Merged

Allow ignoring PRs with specific labels from review#245
dgershman merged 1 commit intomainfrom
feature/crow-244-ignore-labels-review

Conversation

@dgershman
Copy link
Copy Markdown
Collaborator

Closes #244

Summary

  • Adds ignoreReviewLabels configuration option (comma-separated label names) to filter PRs from the review board
  • Fetches PR labels via the GitHub GraphQL query (labels(first: 20) { nodes { name } })
  • Applies case-insensitive label matching alongside the existing repo-exclude filter
  • PRs with any matching label are hidden from the review board, badge counts, and notifications
  • Settings UI in Automation → Reviews mirrors the existing "Excluded Repos" pattern

Test plan

  • CrowCore package builds cleanly
  • All 143 unit tests pass (including 2 new config round-trip tests)
  • Manual: configure ignoreReviewLabels: ["dependencies"] and verify matching PRs are filtered from the review board
  • Manual: verify existing PRs without matching labels still appear

🤖 Generated with Claude Code

Add `ignoreReviewLabels` config option that filters PRs bearing any of
the listed labels from the review board, badge counts, and notifications.
Fetches labels via the GitHub GraphQL query and applies case-insensitive
matching alongside the existing repo-exclude filter.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown
Contributor

@dhilgaertner dhilgaertner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code & Security Review

Security Review

Strengths:

  • No new attack surface. The GraphQL change just adds labels(first: 20) { nodes { name } } to an existing hardcoded query — values flow through gh api graphql -F parameters, not string interpolation, so no injection vector.
  • Label parsing in IssueTracker.swift:1106-1107 is type-safe (compactMap { $0["name"] as? String }) with a sensible empty-array fallback.
  • All filtering is local; the user-configured label list is never sent to a remote system.

Concerns:

  • None.

Code Quality

Strengths:

  • Backwards-compatible config decoding — missing ignoreReviewLabels key falls back to [], covered by ignoreReviewLabelsDefaultsEmptyWhenKeyMissing test (AppConfigTests.swift:261-265).
  • Filter pattern mirrors the existing excludeReviewRepos flow exactly: applied in IssueTracker.swift:307-313 before storing into appState.reviewRequests, and again in AppState.filteredReviewRequests for defense in depth. Consistency is good.
  • Notification dedup correctness preserved: previousReviewRequestIDs = allCurrentIDs is set from the unfiltered ID set, so a PR transitioning between hidden/visible (label added or removed) doesn't spuriously fire as a "new review request" toast (IssueTracker.swift:302,316).
  • Case-insensitive label match aligns with the repo-pattern matching convention.
  • 17 lines of new tests including round-trip and missing-key cases.

Considerations (non-blocking):

  • labels(first: 20) in IssueTracker.swift:572 — a PR with more than 20 labels would silently truncate beyond the cap; if a configured ignore-label is the 21st on a noisy PR it won't be filtered. Almost certainly fine in practice; just worth being aware of.
  • Labels are now fetched on every review-PR refresh even when ignoreReviewLabels is empty — payload-size cost is trivial, but notable.
  • The settings TextField is initialized from defaults.wrappedValue.ignoreReviewLabels once at init time (AutomationSettingsView.swift:31); external mutations to defaults won't update the visible text. This matches the pre-existing pattern for excludeReviewReposText, so not a regression.
  • ReviewRequest.labels is only populated on the GitHub path (IssueTracker.swift:1109-1122); there's no GitLab MR equivalent, but GitLab review requests aren't currently surfaced through reviewRequests anyway, so nothing breaks.

Build & Tests

  • swift build against Packages/CrowCore succeeds clean.
  • swift test passes all 143 tests in 5 suites (including the 2 new ones).
  • The umbrella swift build from repo root fails on a missing Frameworks/GhosttyKit.xcframework binary artifact — unrelated to this PR's changes.

Summary Table

Priority Issue
🟢 Consider surfacing why a PR is hidden in the review board UI (e.g., a "filtered by label" affordance), so users don't wonder where their PRs went.
🟢 Consider widening labels(first: 20) or paginating if any tracked repos use heavy label taxonomies.

Recommendation: Approve. Tightly scoped, follows existing patterns, backwards-compatible, well tested. No security or correctness concerns.

@dgershman dgershman merged commit c3c69cb into main May 7, 2026
3 checks passed
@dgershman dgershman deleted the feature/crow-244-ignore-labels-review branch May 7, 2026 03:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Allow ignoring PRs with specific labels from review

2 participants