fix(review,init): single severity vocab, BASE_REF capture, command -v parallel-cancel#67
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates the Ruby/Grape/Rails Claude Code plugin’s review/init instructions and release metadata for v1.16.13, focusing on review severity vocabulary consistency, safer base-ref capture guidance, and tool detection behavior.
Changes:
- Standardizes review severity wording around
blocker | warning | suggestionfor workers andBLOCKER | WARNING | SUGGESTIONfor consolidated output. - Moves
/rb:reviewbase-ref/diff collection guidance into the skill body and adds artifact-reading guidance. - Bumps release metadata to
1.16.13with a matching changelog entry.
Reviewed changes
Copilot reviewed 11 out of 12 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
plugins/ruby-grape-rails/skills/review/SKILL.md |
Updates review collection, fanout, severity, and artifact-reading instructions. |
plugins/ruby-grape-rails/skills/review/references/review-playbook.md |
Aligns worker severity mapping and synthesis guidance with the new vocabulary. |
plugins/ruby-grape-rails/skills/init/SKILL.md |
Replaces multi-arg command -v guidance with independent tool probing. |
plugins/ruby-grape-rails/references/preferences/tool-batching.md |
Adds BAD/GOOD guidance for multi-tool command -v detection. |
plugins/ruby-grape-rails/agents/migration-safety-reviewer.md |
Updates sample severity categories to blocker/warning/suggestion. |
plugins/ruby-grape-rails/agents/iron-law-judge.md |
Updates violation/severity examples and confidence casing. |
plugins/ruby-grape-rails/agents/data-integrity-reviewer.md |
Updates sample severity categories to blocker/warning/suggestion. |
plugins/ruby-grape-rails/.claude-plugin/plugin.json |
Bumps plugin version to 1.16.13. |
package.json |
Bumps package version to 1.16.13. |
package-lock.json |
Bumps lockfile package versions to 1.16.13. |
CHANGELOG.md |
Adds the 1.16.13 release notes and version links. |
.claude-plugin/marketplace.json |
Bumps marketplace metadata and plugin ref to v1.16.13. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 16 out of 17 changed files in this pull request and generated 9 comments.
Comments suppressed due to low confidence (1)
plugins/ruby-grape-rails/agents/verification-runner.md:105
- This second Pronto instruction has the same shell-scope problem:
$BASE_REFonly exists in the shell that evaluatedresolve-base-ref, so a separate Bash call forbundle exec prontowill run with an empty/unset base ref. Keep the eval and Pronto command in one Bash invocation or state that explicitly.
- If `PRONTO_AVAILABLE=true`, run
`eval "$(${CLAUDE_PLUGIN_ROOT}/bin/resolve-base-ref)"` (populates
`$BASE_REF`, `$REMOTE`, `$DEFAULT_BRANCH`); then
`bundle exec pronto run -c "$(git merge-base HEAD "$BASE_REF")"`
as the optional final diff-scoped step
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 48 out of 50 changed files in this pull request and generated 12 comments.
Comments suppressed due to low confidence (6)
lab/eval/output_checks.py:849
- This second At-a-Glance parser also normalizes with
.title(), so non-canonical severity casing is accepted during the coverage cross-check. Keep this strict so the validator rejects oldBLOCKER/WARNING/SUGGESTIONartifacts under the new title-case contract.
severity = row[2].strip().title()
reviewer = row[4].strip()
new_state = row[6].strip().lower()
if severity not in {"Blocker", "Warning", "Suggestion"}:
bad_enum.append(
f"At-a-Glance row {idx}: `Severity` cell {row[2]!r} not in "
"{`Blocker`, `Warning`, `Suggestion`} — malformed enum"
plugins/ruby-grape-rails/skills/triage/SKILL.md:260
- The phase-output instructions still use the old uppercase
NEW WARNINGs/NEW SUGGESTIONsvocabulary. This conflicts with the new title-case plan/template examples and can train triage to emit mixed casing in generated plans.
plugins/ruby-grape-rails/skills/triage/SKILL.md:294 - This paragraph still uses
BLOCKERseven though the surrounding shortcut labels were converted to title case. Use the sameBlocker/Blockersvocabulary here so the selection UI instructions do not mix old and new bucket names.
plugins/ruby-grape-rails/skills/review/SKILL.md:354 - This repeats the new bulk-
cat/Readrule a second time in the same shipped skill body. Keep the detailed tool-batching guidance in the preference reference and point there instead of duplicating it in the review workflow text.
plugins/ruby-grape-rails/skills/triage/SKILL.md:360 - The all-blocker edge-case sample still emits
BLOCKERswhile the condition now usesBlocker. This canonical UI text should use the same title-case vocabulary to avoid training/rb:triageto produce mixed casing.
plugins/ruby-grape-rails/skills/full/references/example-run.md:69 - Only this line was converted to
Suggestion; the same example block still uses old uppercaseSUGGESTIONs,BLOCKERs, andWARNINGsnearby. This keeps the shipped/rb:fullexample split across two severity vocabularies despite the new review casing contract.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 48 out of 50 changed files in this pull request and generated 7 comments.
Comments suppressed due to low confidence (3)
plugins/ruby-grape-rails/skills/review/SKILL.md:355
- This Gotcha restates the Read-over-cat/tool-batching discipline inside a shipped skill body. The repo policy requires skill bodies to use pointer-only links to the preference doc rather than duplicating the rule text and rationale.
plugins/ruby-grape-rails/skills/triage/SKILL.md:357 - The heading and condition use singular "Blocker" for a plural set of findings; use the plural form here to match the following "All 5 findings are Blockers" sentence.
plugins/ruby-grape-rails/skills/triage/SKILL.md:298 - The shortcut names use singular nouns with "Select All" even though the operation selects multiple findings; plural labels would read correctly and match the surrounding plural "Blockers" wording.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 48 out of 50 changed files in this pull request and generated 13 comments.
Comments suppressed due to low confidence (2)
plugins/ruby-grape-rails/agents/iron-law-judge.md:101
- The section says all Iron Law violations are Blockers, but the Blocker table below still omits Laws 5, 8, 9, and 17 from the 22-law registry above. Those omitted laws can be treated as lower-priority or missed entirely by this agent despite the new all-Blocker doctrine.
| Law | Pattern | Risk |
|-----|---------|------|
| 1 | `t.float :price` | Financial errors |
| 2, 15 | `where("id = #{id}")` | SQL injection |
| 3 | N+1 queries (loop without `includes`) | Performance / DB load |
plugins/ruby-grape-rails/agents/iron-law-judge.md:158
- This priority summary excludes Law 17 even though the section above says all Iron Law violations are Blockers. Law 17 remains in the canonical registry, so omitting it here creates an ambiguous priority for supervised-background-process violations.
1. **Blockers** (Laws 1-16, 18-20): All Iron Law violations block merge
2. **Verification** (Law 21): Testing discipline — Required
3. **Surgical Changes** (Law 22): Out-of-scope edits flagged separately
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 50 out of 52 changed files in this pull request and generated 12 comments.
Comments suppressed due to low confidence (2)
plugins/ruby-grape-rails/skills/review/references/review-playbook.md:651
- This command is shown as a runnable diff-LOC calculation, but
<MERGE_BASE>is shell redirection syntax when copied into Bash. Use an executable form or move the placeholder substitution instruction outside the command block so agents do not run an invalid command.
lab/eval/output_checks.py:795 - The strict title-case severity enum introduced here is not covered by a regression test that rejects the old uppercase values (
BLOCKER,WARNING,SUGGESTION). Add a negative test so the intended “no backward compat” behavior cannot be accidentally loosened by reintroducing normalization.
severity = row[2].strip()
new_state = row[6].strip().lower()
if severity not in new_counts:
bad_enum.append(
|
@codex review |
|
Codex Review: Didn't find any major issues. 🚀 ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
No description provided.