feat(labels): estate label tooling + auto-triage for new issues - #78
feat(labels): estate label tooling + auto-triage for new issues#78hyperpolymath wants to merge 1 commit into
Conversation
|
Warning Review limit reachedNext included review available in 27 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
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 |
Up to standards ✅🟢 Issues
|
There was a problem hiding this comment.
Pull Request Overview
This PR introduces a robust jq-based label classification and triage system. While the implementation aligns with the architectural requirement to avoid Python and external actions, several critical issues must be addressed before merging.
Notably, although Codacy analysis indicates the PR is 'up to standards', all six required test scenarios identified to verify the triage logic and sync behavior are missing. Furthermore, the triage workflow and the core JQ script are identified as high-complexity files with no test coverage, increasing the risk of regressions in regex handling or label application. A significant logic bug regarding word splitting in Bash was also identified, which will cause failures when handling labels containing spaces (e.g., 'good first issue'). Finally, the .github/workflows/actions.lock file mentioned in the PR description is missing from the diff.
About this PR
- The PR description refers to a parity test suite that exists elsewhere. Given the complexity of the regex logic in the JQ classifier, this test suite or equivalent unit tests should be integrated into the repository to validate the classification logic locally.
- The generator scripts mentioned in the headers of label-classifier.json and labels.json are not included in this PR. To ensure maintainability, these scripts should be part of the repository so the generated artifacts can be updated as the canonical label set evolves.
1 comment outside of the diff
.github/workflows/actions.lock
line 1🟡 MEDIUM RISK
The changes to .github/workflows/actions.lock mentioned in the PR description are missing from the diff. Please ensure this file is included to maintain the security posture of the workflows.
Test suggestions
- Missing: Verify that a title prefix (e.g., 'feat:') results in the correct type label ('enhancement').
- Missing: Verify that bracket tags (e.g., '[p0]') are correctly mapped to priority labels.
- Missing: Verify that the classifier returns no labels if the issue already has a label in the same 'max-1' tier (e.g., already has a 'bug' label).
- Missing: Verify that the label sync workflow creates missing labels defined in labels.json.
- Missing: Verify that the label sync workflow skips updates for labels defined in the 'frozen' list.
- Missing: Verify that the triage workflow remains silent and exits 0 when no confident classification is found.
- Missing: Implement unit tests for the complex regex construction logic in
kwrxwithin .github/scripts/classify-issue.jq.
Prompt proposal for missing tests
Consider implementing these tests if applicable:
1. Missing: Verify that a title prefix (e.g., 'feat:') results in the correct type label ('enhancement').
2. Missing: Verify that bracket tags (e.g., '[p0]') are correctly mapped to priority labels.
3. Missing: Verify that the classifier returns no labels if the issue already has a label in the same 'max-1' tier (e.g., already has a 'bug' label).
4. Missing: Verify that the label sync workflow creates missing labels defined in labels.json.
5. Missing: Verify that the label sync workflow skips updates for labels defined in the 'frozen' list.
6. Missing: Verify that the triage workflow remains silent and exits 0 when no confident classification is found.
7. Missing: Implement unit tests for the complex regex construction logic in `kwrx` within .github/scripts/classify-issue.jq.
TIP Improve review quality by adding custom instructions
TIP How was this review? Give us feedback
|
|
||
| printf 'applying: %s\n' "${apply[*]}" | ||
| gh issue edit "$NUM" -R "$GITHUB_REPOSITORY" \ | ||
| $(printf -- '--add-label %q ' "${apply[@]}") \ |
There was a problem hiding this comment.
🟡 MEDIUM RISK
Word splitting will break if a label contains spaces (e.g., 'good first issue'). This is a high-risk area given the complexity of the triage workflow. Use an array to collect arguments instead:
| $(printf -- '--add-label %q ' "${apply[@]}") \ | |
| args=() | |
| for label in "${apply[@]}"; do args+=("--add-label" "$label"); done | |
| gh issue edit "$NUM" -R "$GITHUB_REPOSITORY" "${args[@]}" \ |
| frozen=0 | ||
| for f in "${FROZEN[@]}"; do [ "$f" = "$name" ] && frozen=1 && break; done | ||
|
|
||
| cur=$(printf '%s\n' "$existing" | awk -F'\t' -v n="$name" '$1==n{print;exit}') |
There was a problem hiding this comment.
🟡 MEDIUM RISK
Suggestion: Spawning awk in a loop creates significant overhead in a fleet-wide workflow. Try running the following prompt in your coding agent:
In .github/workflows/labels.yml, optimize the label lookup logic by parsing the
existingTSV variable into a Bash associative array (declare -A) once before the loop, then look up labels by key inside the loop to avoid spawningawksubshells.
| # (`port` + `ion` = "portion", and `port` is a live keyword). They are enabled | ||
| # only for shapes that are unambiguously truncated stems -- `-at` | ||
| # (instantiat, investigat, adjudicat) and `-ment` (document, implement). | ||
| def kwrx($kw): |
There was a problem hiding this comment.
⚪ LOW RISK
Suggestion: The regex construction logic in kwrx is quite complex and critical for the triage system. Consider adding unit tests for this specific function to ensure that the inflection-tolerant matching doesn't regress.
Try running the following prompt in your IDE agent:
Create a standalone shell script that runs
jqagainstclassify-issue.jqwith a set of test cases (mock issue titles and expected keywords) to verify the regex boundary logic.
| && updated=$((updated+1)) | ||
| fi | ||
| fi | ||
| sleep 0.4 |
There was a problem hiding this comment.
⚪ LOW RISK
Suggestion: Move the sleep command inside the if blocks for gh label create and gh label edit so it only runs when a write API call is actually made, rather than pausing on every iteration.
Ships the canonical label set and the classifier that labels newly-filed issues. Additive only: it never removes a label, never overrides a human's classification, stays silent when unsure, and never fails an issue. Also adds this repo's two new workflows to .github/workflows/actions.lock as '[]'. That lock is keyed by workflow path and refuses any workflow it does not list -- a startup_failure, which produces no check run and is therefore silent. `gh actions-lock` cannot add these: it records action versions, and both workflows deliberately use no actions. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1f4b0ae to
3204819
Compare
Ships the canonical label set and the classifier that labels newly-filed issues.
Additive only — never removes a label, never overrides a human's classification, silent when unsure, never fails an issue.
Also adds this repo's two new workflows to
.github/workflows/actions.lockas[]. That lock is keyed by workflow path and refuses any workflow it does not list — astartup_failure, which produces no check run and is therefore silent.gh actions-lockcannot add these: it records action versions, and both workflows deliberately use none.See
docs/LABELS.adocin hyperpolymath/.git-private-farm.🤖 Generated with Claude Code