feat(ci): add PR path-based label routing and priority recommendation - #573
Conversation
Astro-Han
added
enhancement
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
✅ Files skipped from review due to trivial changes (2)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughAdds path-based labeler rules, a priority-classifier helper script, two GitHub Actions workflows (labeler + triage), and tests that validate workflow metadata and triage outputs. ChangesAutomated PR Routing and Priority Triage
Sequence DiagramsequenceDiagram
participant PR as Pull Request
participant Labeler as labeler workflow
participant LabelerAction as actions/labeler
participant Triage as pr-priority-triage workflow
participant Script as pr-priority-triage.js
participant GitHub as GitHub API
PR->>Labeler: pull_request_target (opened/synchronized/reopened)
Labeler->>LabelerAction: run globs to compute labels
LabelerAction->>GitHub: set labels (ci/platform/app/ui/harness/documentation)
PR->>Triage: pull_request_target (opened/synchronized/reopened)
Triage->>Script: list PR files and existing reviews
Script->>Script: classifyPriority(changed_paths)
Script->>GitHub: detect existing TRIAGE_MARKER comment
Script->>Script: buildPriorityReview(paths)
Script->>GitHub: post COMMENT review with priority verdict
Triage->>GitHub: log posted priority
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested labels
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Astro-Han
left a comment
Astro-Han
left a comment
There was a problem hiding this comment.
Opus second-pass review — PR #573
Pass, no blockers.
Strengths
- Security pattern correct:
pull_request_target+actions/checkoutwithref: github.event.pull_request.base.shaensures the workflow never runs PR-from-fork head code while still having access to the repo token. This is the recommended pattern for triage workflows. persist-credentials: falseis fine here because both labeler and the github-script step use the API directly, notgit push— no auth wiring needed.- Concurrency group
pr-priority-triage-${pr.number}withcancel-in-progress: truecorrectly handles rapid pushes, and theTRIAGE_MARKERdedup ensures duplicate comments cannot happen even with race-cancel. - All third-party actions pinned to commit SHA (
actions/checkout@de0fac2e...,actions/github-script@f28e40c7...,actions/labeler@8558fd74...), no@v6floating tags. - Custom glob-to-regex in
pr-priority-triage.jscorrectly handles**/prefix ((?:.*/)?) and bare**(.*). Traceddocs/**,**/*.md, andpackages/app/src/**— all behave as expected. - Conservative v1 design: never auto-applies a P label, always comments a recommendation, P1/P0 reserved for maintainer. This is exactly the right trust model.
- Deliberate deviation from issue body: GPT chose real
platform/harness/documentationlabels over the issue body's placeholderdesktop/opencode/docsbecause those don't exist in the repo. Documented in PR body. This is correct judgment — the workflow would have succeeded silently but applied no labels otherwise. sync-labels: trueis safe in this setup because the labeler only manages the 6 path-based labels declared inlabeler.yml; maintainer-added P labels and other labels (release-gate, windows, flaky-test, etc.) are untouched.- Sanity cases in the contract test against recent real PRs are convincing:
- 7 contract tests cover the classifier paths, marker dedup, and glob edge cases.
Nit (no action)
-
pull_request_targetwithsynchronizere-runs on every push, but the marker dedup means subsequent pushes do NOT update the recommendation if the scope changes drastically (e.g., a PR that started doc-only and grew to product code). Acceptable for v1 — the comment is advisory and maintainers can manually relabel. If this becomes a pain point, a future iteration could compare current paths to the marker-encoded snapshot and re-post when scope drifts beyond a threshold. -
Could add an explicit note in PR body that
sync-labels: trueis intentionally limited to path-based labels and won't touch P-level or other manual labels. Helps future maintainers who scan the workflow and worry about label collisions.
Verdict
CI workflow + path-based labeler + advisory comment surface. No product code, no release workflow change, no UI/UX. Conservative v1 design with sound security pattern and good test coverage. Ready for engineering final from @GPT-X.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces an automated PR priority triage system, including a labeler configuration and a script to classify PRs as P2 or P3 based on changed file paths. Feedback focused on optimizing the triage script by memoizing compiled regular expressions and truncating the list of file paths in the triage reason to avoid hitting GitHub comment size limits and reduce noise.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
.github/labeler.yml (1)
16-21: ⚡ Quick winConsider narrowing the
**/*.tsxglob to exclude test files.The
**/*.tsxpattern will match all TypeScript JSX files in the repository, including test files (e.g.,packages/app/e2e/**/*.tsx,**/test/**/*.tsx). This may dilute the signal if theuilabel is intended primarily for production UI component changes.If test TSX files should not be labeled as
ui, consider adding exclusion patterns or constraining the glob to production directories.🎯 Example: Exclude test directories from ui label
ui: - changed-files: - any-glob-to-any-file: - "packages/app/src/components/**" - "packages/ui/**" - - "**/*.tsx" + - "packages/app/src/**/*.tsx" + - "packages/desktop-electron/src/**/*.tsx"Note: Verify whether story files, E2E specs, or other .tsx files in test directories should also receive the
uilabel before applying this change.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/labeler.yml around lines 16 - 21, The ui label's broad "**/*.tsx" glob will match test/spec files; update the ui -> changed-files -> any-glob-to-any-file entry to exclude test paths by removing or constraining the "**/*.tsx" pattern and instead either target production dirs (e.g., package src and ui folders already listed) or add exclusion patterns such as excluding "**/test/**", "**/e2e/**", and filename patterns like "**/*.spec.tsx" and "**/*.test.tsx"; modify the pattern(s) around the existing ui and any-glob-to-any-file entries so only intended production UI .tsx files trigger the label.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In @.github/labeler.yml:
- Around line 16-21: The ui label's broad "**/*.tsx" glob will match test/spec
files; update the ui -> changed-files -> any-glob-to-any-file entry to exclude
test paths by removing or constraining the "**/*.tsx" pattern and instead either
target production dirs (e.g., package src and ui folders already listed) or add
exclusion patterns such as excluding "**/test/**", "**/e2e/**", and filename
patterns like "**/*.spec.tsx" and "**/*.test.tsx"; modify the pattern(s) around
the existing ui and any-glob-to-any-file entries so only intended production UI
.tsx files trigger the label.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 356ab09b-9a40-4b94-8ac4-fd640a1839cf
📒 Files selected for processing (5)
.github/labeler.yml.github/scripts/pr-priority-triage.js.github/workflows/labeler.yml.github/workflows/pr-priority-triage.ymlpackages/opencode/test/github/pr-routing-triage.test.ts
3146e16 to
13f95f1
Compare
1 participant
Summary
Add two conservative PR triage automations:
actions/labeleractions/github-script, posting a review comment instead of auto-applyingP*labelsThis keeps maintainer control over
P0/P1while reducing manual relabel work for routine PRs.Why
Issue #135 asks for a first-pass PR queue that is easier to scan without turning priority into an opaque bot decision. The safe v1 boundary is:
P3only for obvious low-risk PRs like docs, workflows, tests, and e2e-only changesP2for app/desktop user-path changes and other non-low-risk pathsP1/P0I also aligned the path routing with the existing repo label taxonomy instead of inventing new labels. Concretely:
.github/workflows/**->cipackages/desktop-electron/**->platformpackages/app/**->apppackages/app/src/components/**,packages/ui/**,**/*.tsx->uipackages/opencode/**->harnessdocs/**,**/*.md->documentationThat preserves current label vocabulary while still matching the issue's routing intent.
Related Issue
Closes #135.
Human Review Status
Pending. A human should make the final merge decision after reviewing the final diff and verification evidence.
Review Focus
P2/P3recommendations, no auto-priority labels, no reviewer assignment.opened,synchronize, andreopenedwithout creating repeat triage noise.Risk Notes
pull_request_target, but it checks out the PR base SHA only. It does not execute code from the PR head.P*labels. That keeps maintainer override simple and avoids accidentalP1/P0escalation.actions/labeleris configured against current repo labels (platform,harness,documentation) rather than the issue's placeholder names (desktop,opencode,docs) because those placeholder labels do not exist in the repo today.How To Verify
Screenshots or Recordings
Not needed. This is CI automation with no visible UI change.
Checklist
dev, and my PR title and commit messages use Conventional Commits in EnglishSummary by CodeRabbit
Chores
Tests