Skip to content

feat: nextsteps-specific PR review skill (NES-1498)#8936

Merged
jaco-brink merged 9 commits intomainfrom
jacobusbrink/nes-1498-nextsteps-specific-pr-review-skill
Apr 1, 2026
Merged

feat: nextsteps-specific PR review skill (NES-1498)#8936
jaco-brink merged 9 commits intomainfrom
jacobusbrink/nes-1498-nextsteps-specific-pr-review-skill

Conversation

@jaco-brink
Copy link
Copy Markdown
Collaborator

@jaco-brink jaco-brink commented Mar 31, 2026

Overview

Adds a review-pr skill (both Claude Code and Cursor) that auto-loads project conventions when reviewing PRs. When a PR touches files in apps/journeys-admin/, the skill loads apps/journeys-admin/AGENTS.md and the matching .claude/rules/ — making it aware of MUI-only, i18n, provider stack, and testing conventions. Same for APIs (Prisma patterns, Pothos conventions, the customizable blocks sync guardrail, ACL parity rule).

What's added:

  • .claude/commands/review-pr.md — Claude Code command
  • .cursor/skills/review-pr/SKILL.md — Cursor skill
  • apps/journeys/AGENTS.md — Convention guide (component patterns, providers, i18n, block model)
  • apps/journeys-admin/AGENTS.md — Convention guide (editor architecture, hooks, test patterns)
  • apis/AGENTS.md — Convention guide covering api-journeys, api-journeys-modern, api-gateway

Key mechanism — Step 1.6 (convention loading):

  1. Extracts changed file paths from the PR diff
  2. Matches against .claude/rules/ and .cursor/rules/ path patterns
  3. Walks up directory trees to find AGENTS.md files
  4. Injects matched conventions as review context
  5. Convention violations become Concern minimum; guardrail violations (e.g., customizable blocks sync) become Critical

Falls back to generic review when no conventions match.

Evaluation

We evaluated the skill against 3 recently merged PRs with real human review feedback:

PR Type Human comments Skill new findings Genuinely valuable
#8882 (journeys-admin) Race condition fix 4 (csiyang) 4 2
#8883 (API migration) Mutation migration 0 5 1
#8896 (ACL security fix) Security fix 0 4 2

Strongest result — PR #8896: The apis/AGENTS.md ACL parity section caused the skill to cross-check the legacy API when the modern API's ACL was changed. No human reviewer or bot did this. It also identified that read() and journeyReadAccessWhere() have no shared role constant — the same class of drift that caused the original bug.

Weakest result — PR #8883: Most findings were "correct analysis, wrong context" — flagging established patterns on a routine migration PR.

Overall: 38% of new findings were genuinely valuable, 31% were noise (nit-level). The valuable findings were things humans missed. Noise is filterable via the skill's triage confirmation step.

Full evaluation with per-finding triage: NES-1498 comment

Test plan

  • Review AGENTS.md files for accuracy against actual codebase patterns
  • Run /review-pr mode=local on a current open PR to validate convention loading
  • Verify the skill correctly loads rules for mixed-area PRs (e.g., frontend + API changes)

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Documentation
    • Added centralized API conventions and project-specific development guides for frontend apps, covering stacks, component structure, naming, UI/accessibility, testing, and quality gates.
    • Added a comprehensive PR review workflow and companion skill: local preview vs remote-posting modes, triage classifications (Critical/Concern/Nit/Skip), project-rule loading, large-PR scoping/prioritization, deduplication, and confirmation vs auto-run behavior.

…NES-1498)

Add review-pr skill (.claude/commands/ and .cursor/skills/) with dynamic
convention loading — Step 1.6 loads matching rules and AGENTS.md files
based on changed file paths, making reviews aware of project conventions.

Add AGENTS.md convention guides for:
- apps/journeys/ (component patterns, providers, i18n, block model)
- apps/journeys-admin/ (editor architecture, 60+ hooks, test patterns)
- apis/ (api-journeys, api-journeys-modern, api-gateway — Prisma, Pothos,
  customizable blocks sync guardrail, ACL parity rule)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@jaco-brink jaco-brink self-assigned this Mar 31, 2026
@jaco-brink jaco-brink requested a review from jianwei1 March 31, 2026 01:12
@linear
Copy link
Copy Markdown

linear Bot commented Mar 31, 2026

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Mar 31, 2026

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

Walkthrough

Adds a documented PR review command/skill (parameterized pr/mode/auto) with thread deduplication, PR scoping, rule/AGENTS.md-based conventions loading, triage and confirmation flows, remote/local posting modes; and three new AGENTS convention guides for APIs, journeys-admin, and journeys apps.

Changes

Cohort / File(s) Summary
PR Review Workflow
​.claude/commands/review-pr.md, .cursor/skills/review-pr/SKILL.md
Adds a comprehensive PR review specification/skill: PR discovery, diff & metadata fetch, paginated GraphQL retrieval of reviewThreads for deduplication, PR-size scoping (Deep/Scan/Skim), convention loading via .claude/.cursor rules and AGENTS.md ancestry, triage taxonomy (Critical/Concern/Nit/Skip), confirmation gate (auto), and remote vs local posting behaviors.
API Conventions
apis/AGENTS.md
New API conventions document for GraphQL federation services: shared Prisma patterns and commands, migration and soft-delete rules, federation guidance (@key, @shareable, Federation 2.6), two cross-cutting guardrails (customizable-blocks recalculation and ACL parity), and repository quality-gate commands.
Frontend App Conventions
apps/journeys-admin/AGENTS.md, apps/journeys/AGENTS.md
Adds app-specific AGENTS guides covering stack choices (Next.js Pages Router, MUI, Apollo, Firebase, i18n), component folder/naming conventions, provider wrapping order, GraphQL/codegen and data-fetching norms, routing/i18n rules, testing/provider requirements, accessibility/UI constraints, and quality-gate commands.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant CLI as CLI / Operator
  participant Skill as Review Skill
  participant Repo as Repo (rules & AGENTS.md)
  participant GH as GitHub API
  participant Operator as Operator

  CLI->>Skill: invoke review-pr(pr?, mode, auto)
  Skill->>Repo: load changed files, matching rules & AGENTS.md
  Skill->>GH: fetch PR diff & metadata
  Skill->>GH: query existing review threads (paginated GraphQL)
  Skill->>Skill: scope review (Deep / Scan / Skim) and apply conventions
  Skill->>Skill: triage findings (Critical / Concern / Nit / Skip)
  alt auto = false
    Skill->>Operator: present "PR Review Triage" for confirmation
    Operator-->>Skill: confirm / modify
  end
  alt mode = remote
    Skill->>GH: submit single inline-comment review (event based on triage & draft status)
  else mode = local
    Skill-->>CLI: render findings locally (option: "post it" to remote later)
  end
Loading

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~2 minutes

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'feat: nextsteps-specific PR review skill (NES-1498)' accurately and concisely describes the main addition: a review skill tailored for the NextSteps project with area-specific conventions and guardrails.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch jacobusbrink/nes-1498-nextsteps-specific-pr-review-skill

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

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

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Mar 31, 2026

Warnings
⚠️ ❗ Big PR (1025 changes)

(change count - 1025): Pull Request size seems relatively large. If Pull Request contains multiple changes, split each into separate PR will helps faster, easier review.

Generated by 🚫 dangerJS against 2547b45

@nx-cloud
Copy link
Copy Markdown

nx-cloud Bot commented Mar 31, 2026

View your CI Pipeline Execution ↗ for commit 2547b45

Command Status Duration Result
nx run journeys-e2e:e2e ✅ Succeeded 19s View ↗
nx run journeys-admin-e2e:e2e ✅ Succeeded 29s View ↗
nx run-many --target=vercel-alias --projects=jo... ✅ Succeeded 2s View ↗
nx run-many --target=upload-sourcemaps --projec... ✅ Succeeded 9s View ↗
nx run-many --target=deploy --projects=journeys... ✅ Succeeded 2m 47s View ↗
nx run-many --target=vercel-alias --projects=jo... ✅ Succeeded 2s View ↗
nx run-many --target=upload-sourcemaps --projec... ✅ Succeeded 4s View ↗
nx run-many --target=deploy --projects=journeys ✅ Succeeded 1m 13s View ↗

☁️ Nx Cloud last updated this comment at 2026-04-01 05:05:41 UTC

@github-actions github-actions Bot requested a deployment to Preview - journeys March 31, 2026 01:13 Pending
@github-actions github-actions Bot requested a deployment to Preview - journeys-admin March 31, 2026 01:13 Pending
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 6

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In @.claude/commands/review-pr.md:
- Around line 134-154: Add a language hint to the fenced triage example block
that begins with "## PR Review Triage — PR `#123`: <title>" by changing the
opening ``` to a language-specific fence such as ```markdown (or ```text) so
markdownlint passes; keep the closing ``` unchanged and apply the same change in
the suggested doc fix snippet.
- Around line 36-66: The GraphQL query for reviewThreads only fetches the first
page and can miss older threads; update the logic around the query (the GraphQL
call that asks for repository.pullRequest.reviewThreads and reads
pageInfo.hasNextPage/endCursor) to iterate pages: when pageInfo.hasNextPage is
true, re-run the query with an after:$cursor variable using endCursor, append
nodes to a cumulative reviewThreads list, and only run the existing
deduplication/posting logic after all pages are fetched so duplicates are
avoided.

In @.cursor/skills/review-pr/SKILL.md:
- Around line 141-159: The fenced code block containing the PR triage example is
unlabeled and triggers MD040; fix it by adding a language tag (e.g., change the
leading ``` to ```markdown or ```text) for the fence that surrounds the "## PR
Review Triage — PR `#123`: <title>" example so the block is properly recognized;
update the same fence in the suggested doc fix snippet (replace ``` with
```markdown) to keep both examples consistent.
- Around line 42-73: The current GraphQL fetch only reads the first 100
reviewThreads and must be converted to cursor-based pagination: change the query
to accept a $cursor (nullable) and pass it as after: $cursor into reviewThreads,
then loop while pageInfo.hasNextPage is true by re-running the query with
$cursor set to pageInfo.endCursor, accumulating reviewThreads.nodes across
iterations; after collecting all nodes run the existing deduplication logic.
Ensure you update the query variables and the caller loop that invokes
reviewThreads to use $cursor, and reference the pageInfo.hasNextPage and
pageInfo.endCursor fields to drive the loop.

In `@apis/AGENTS.md`:
- Around line 181-183: The doc's "Editable text fields" guardrail is
incorrect—update the APIS.md wording to match the implementation used in
journeyCustomizable.service.ts and recalculateJourneyCustomizable.ts: state that
editable text is determined by journeyCustomizationDescription != null
(non-empty/non-null) rather than requiring journeyCustomizationFields count > 0,
and remove or clarify the requirement about journeyCustomizationFields to avoid
false-positive critical reviews.

In `@apps/journeys-admin/AGENTS.md`:
- Around line 23-32: The fenced code blocks showing directory examples (e.g.,
the blocks containing "ComponentName/", "Editor/", and "src/libs/") are
unlabeled and trigger MD040; update each triple-backtick fence to include a
language identifier (use text for plain listings or bash if showing commands) so
the blocks become ```text or ```bash accordingly, ensuring all occurrences
(including the other examples noted around the Editor/ and src/libs/ listings)
are consistently labeled.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 1baaa53f-15cb-4f98-a366-bcdae7458dff

📥 Commits

Reviewing files that changed from the base of the PR and between 26f9ef6 and 426408f.

📒 Files selected for processing (5)
  • .claude/commands/review-pr.md
  • .cursor/skills/review-pr/SKILL.md
  • apis/AGENTS.md
  • apps/journeys-admin/AGENTS.md
  • apps/journeys/AGENTS.md

Comment thread .claude/commands/review-pr.md
Comment thread .claude/commands/review-pr.md Outdated
Comment thread .cursor/skills/review-pr/SKILL.md
Comment thread .cursor/skills/review-pr/SKILL.md Outdated
Comment thread apis/AGENTS.md
Comment thread apps/journeys-admin/AGENTS.md Outdated
@github-actions github-actions Bot requested a deployment to Preview - journeys-admin March 31, 2026 01:17 Pending
@github-actions github-actions Bot requested a deployment to Preview - journeys March 31, 2026 01:18 Pending
@github-actions github-actions Bot temporarily deployed to Preview - journeys-admin March 31, 2026 01:19 Inactive
@github-actions github-actions Bot temporarily deployed to Preview - journeys March 31, 2026 01:19 Inactive
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Mar 31, 2026

The latest updates on your projects.

Name Status Preview Updated (UTC)
journeys ✅ Ready journeys preview Wed Apr 1 18:00:28 NZDT 2026

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

🧹 Nitpick comments (2)
apis/AGENTS.md (1)

18-18: Consider softening the soft-delete filtering requirement.

The guidance says "Always filter where: { deletedAt: null }" but some legitimate queries need to access soft-deleted records (admin views, audit logs, restore operations). Consider rephrasing to "Filter where: { deletedAt: null } in user-facing queries" or "Typically filter...".

📝 Suggested rewording
-- Soft deletes via `deletedAt` field on blocks, journeys, and other entities. Always filter `where: { deletedAt: null }` in queries.
+- Soft deletes via `deletedAt` field on blocks, journeys, and other entities. Filter `where: { deletedAt: null }` in user-facing queries (admin/audit queries may need to access soft-deleted records).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apis/AGENTS.md` at line 18, Update the guidance sentence "Soft deletes via
`deletedAt` field on blocks, journeys, and other entities. Always filter `where:
{ deletedAt: null }` in queries." to soften the mandate—e.g., reword to
"Typically filter `where: { deletedAt: null }` in user-facing queries; allow
explicit access to soft-deleted records for admin views, audit logs, and restore
operations." Ensure the new phrasing replaces the original sentence in AGENTS.md
and preserves the mention of the `deletedAt` field and example use-cases
(admin/audit/restore).
.claude/commands/review-pr.md (1)

211-211: Clarify placeholder substitution in the API call.

The command uses literal OWNER, REPO, and NUMBER placeholders, but the substitution from the values resolved in lines 168-169 isn't explicitly shown. Consider adding a note that these placeholders should be replaced with the actual values from the earlier gh repo view and PR number, or show the command with variable interpolation (e.g., repos/$OWNER/$REPO/pulls/$NUMBER/reviews).

📝 Suggested clarification
-| gh api repos/OWNER/REPO/pulls/NUMBER/reviews --method POST --input -
+| gh api repos/$OWNER/$REPO/pulls/$NUMBER/reviews --method POST --input -

Or add a preceding comment:

# Substitute OWNER, REPO from line 168 and NUMBER from the PR parameter
gh api repos/$OWNER/$REPO/pulls/$NUMBER/reviews --method POST --input -
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.claude/commands/review-pr.md at line 211, Update the gh API invocation to
clarify that OWNER, REPO and NUMBER are placeholders by either replacing the
literal placeholders with variable interpolation (e.g.,
repos/$OWNER/$REPO/pulls/$NUMBER/reviews) or adding a preceding comment that
instructs the user to substitute OWNER and REPO from the earlier gh repo view
output and NUMBER from the PR parameter; reference the command shown as "gh api
repos/OWNER/REPO/pulls/NUMBER/reviews --method POST --input -" so reviewers can
locate and update that exact line.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In @.claude/commands/review-pr.md:
- Line 211: Update the gh API invocation to clarify that OWNER, REPO and NUMBER
are placeholders by either replacing the literal placeholders with variable
interpolation (e.g., repos/$OWNER/$REPO/pulls/$NUMBER/reviews) or adding a
preceding comment that instructs the user to substitute OWNER and REPO from the
earlier gh repo view output and NUMBER from the PR parameter; reference the
command shown as "gh api repos/OWNER/REPO/pulls/NUMBER/reviews --method POST
--input -" so reviewers can locate and update that exact line.

In `@apis/AGENTS.md`:
- Line 18: Update the guidance sentence "Soft deletes via `deletedAt` field on
blocks, journeys, and other entities. Always filter `where: { deletedAt: null }`
in queries." to soften the mandate—e.g., reword to "Typically filter `where: {
deletedAt: null }` in user-facing queries; allow explicit access to soft-deleted
records for admin views, audit logs, and restore operations." Ensure the new
phrasing replaces the original sentence in AGENTS.md and preserves the mention
of the `deletedAt` field and example use-cases (admin/audit/restore).

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 6cd9e3be-0e78-4766-90f2-af018b48fae4

📥 Commits

Reviewing files that changed from the base of the PR and between 426408f and abae21f.

📒 Files selected for processing (2)
  • .claude/commands/review-pr.md
  • apis/AGENTS.md

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Mar 31, 2026

The latest updates on your projects.

Name Status Preview Updated (UTC)
journeys-admin ✅ Ready journeys-admin preview Wed Apr 1 18:02:01 NZDT 2026

Copy link
Copy Markdown
Collaborator Author

@jaco-brink jaco-brink left a comment

Choose a reason for hiding this comment

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

Review summary

Concerns: 2 items (see inline)
Nit: 1 item (see inline)
Overall: Good work — comprehensive convention guides and a well-evaluated skill. A couple of maintenance and accuracy items to consider before merge.

Comment thread .claude/commands/review-pr.md Outdated
Comment thread apis/AGENTS.md
Comment thread apis/AGENTS.md
- Add cross-reference sync comments between review-pr command and skill
- Add pagination guidance for GraphQL reviewThreads queries
- Add language identifiers to unlabeled fenced code blocks (MD040)
- Remove hardcoded block type counts from apis/AGENTS.md to avoid drift

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@github-actions github-actions Bot requested a deployment to Preview - journeys-admin March 31, 2026 02:19 Pending
@github-actions github-actions Bot temporarily deployed to Preview - journeys March 31, 2026 02:19 Inactive
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

♻️ Duplicate comments (1)
.cursor/skills/review-pr/SKILL.md (1)

145-163: ⚠️ Potential issue | 🟡 Minor

Add language identifier to the triage example fence.

The code fence starting at line 145 lacks a language identifier, which triggers markdownlint MD040. Adding markdown or text will fix this.

🔧 Suggested fix
-```
+```markdown
 ## PR Review Triage — PR `#123`: <title>
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.cursor/skills/review-pr/SKILL.md around lines 145 - 163, The markdown code
fence in the SKILL.md triage example is missing a language identifier
(triggering markdownlint MD040); update the opening triple-backtick that
precedes the "## PR Review Triage — PR `#123`: <title>" block to include a
language (e.g., change ``` to ```markdown or ```text) so the fenced block is
explicitly typed and the linter warning is resolved.
🧹 Nitpick comments (1)
apis/AGENTS.md (1)

15-15: Minor clarification: "no raw SQL" exception could be more precise.

The phrase "no raw SQL (except prisma.$queryRaw when necessary)" might confuse readers since prisma.$queryRaw is raw SQL. Consider rephrasing to make the guideline clearer.

📝 Suggested rewording
-- ORM for all database access — **no raw SQL** (except `prisma.$queryRaw` when necessary).
+- ORM for all database access via Prisma methods — use `prisma.$queryRaw` only when Prisma's query builder is insufficient.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apis/AGENTS.md` at line 15, The guideline currently says "no raw SQL (except
`prisma.$queryRaw` when necessary)" which is misleading because
`prisma.$queryRaw` is raw SQL; update the sentence to clearly state that raw SQL
should be avoided and only used in exceptional cases via `prisma.$queryRaw` for
queries that cannot be expressed with the ORM, e.g., "Use the ORM for all
database access; avoid raw SQL except in exceptional cases where you must use
prisma.$queryRaw." and ensure the text references `prisma.$queryRaw` as the
explicit escape hatch.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Duplicate comments:
In @.cursor/skills/review-pr/SKILL.md:
- Around line 145-163: The markdown code fence in the SKILL.md triage example is
missing a language identifier (triggering markdownlint MD040); update the
opening triple-backtick that precedes the "## PR Review Triage — PR `#123`:
<title>" block to include a language (e.g., change ``` to ```markdown or
```text) so the fenced block is explicitly typed and the linter warning is
resolved.

---

Nitpick comments:
In `@apis/AGENTS.md`:
- Line 15: The guideline currently says "no raw SQL (except `prisma.$queryRaw`
when necessary)" which is misleading because `prisma.$queryRaw` is raw SQL;
update the sentence to clearly state that raw SQL should be avoided and only
used in exceptional cases via `prisma.$queryRaw` for queries that cannot be
expressed with the ORM, e.g., "Use the ORM for all database access; avoid raw
SQL except in exceptional cases where you must use prisma.$queryRaw." and ensure
the text references `prisma.$queryRaw` as the explicit escape hatch.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: a537ed5c-443e-4522-8c8c-6bec0b59794f

📥 Commits

Reviewing files that changed from the base of the PR and between abae21f and 0a10ec5.

📒 Files selected for processing (4)
  • .claude/commands/review-pr.md
  • .cursor/skills/review-pr/SKILL.md
  • apis/AGENTS.md
  • apps/journeys-admin/AGENTS.md
✅ Files skipped from review due to trivial changes (1)
  • apps/journeys-admin/AGENTS.md

@jaco-brink
Copy link
Copy Markdown
Collaborator Author

Review feedback addressed ('"$COMMIT_SHA"')

Fixed:

  • review-pr.md:7 + SKILL.md:7 — Added cross-reference sync comments between the two review-pr files
  • review-pr.md:66 + SKILL.md:73 — Added pagination guidance for GraphQL reviewThreads queries
  • review-pr.md:154 + SKILL.md:159 — Added markdown language hint to triage example fenced code blocks
  • apps/journeys-admin/AGENTS.md:23,86,108 — Added text language identifier to directory structure fenced code blocks
  • apis/AGENTS.md:66,109 — Removed hardcoded block type counts to avoid drift between api-journeys (27) and api-journeys-modern (26)

Challenged:

  • apis/AGENTS.md:180 (CodeRabbit) — False positive. Both implementations check description.trim().length > 0 && fieldsCount > 0, matching the AGENTS.md documentation. See inline reply.
  • apis/AGENTS.md:173 (jaco-brink) — Verified against source. Documentation is accurate. See inline reply.

@github-actions github-actions Bot requested a deployment to Preview - journeys March 31, 2026 02:23 Pending
@github-actions github-actions Bot requested a deployment to Preview - journeys-admin March 31, 2026 02:23 Pending
@github-actions github-actions Bot temporarily deployed to Preview - journeys March 31, 2026 02:25 Inactive
@github-actions github-actions Bot temporarily deployed to Preview - journeys-admin March 31, 2026 02:25 Inactive
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (1)
.claude/commands/review-pr.md (1)

51-52: Add pagination support or document the comment limit in the nested query.

The nested comments(first:30) query on line 51 lacks pagination fields (pageInfo, endCursor, after), while the parent reviewThreads query includes full pagination support (lines 42-43). If a review thread has more than 30 comments, the deduplication logic (line 70) may not account for older comments beyond the first 30. Either extend the nested query with pagination fields to match the parent, or add a comment explaining why the 30-comment limit is intentional and whether it poses a risk for deduplication.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.claude/commands/review-pr.md around lines 51 - 52, The nested GraphQL field
comments(first:30) lacks pagination and can miss older comments used by your
deduplication logic; either extend that nested selection to include pageInfo
(endCursor, hasNextPage) and accept an after variable (e.g., comments(first:30,
after:$commentsAfter)) so callers can page through comments to match the parent
reviewThreads pagination, or add an explicit inline comment next to
comments(first:30) documenting that the 30-comment cap is intentional and
describing the deduplication risk; update any calling code that relies on the
comments selection (the deduplication logic referenced in the review) to use the
paginated cursors if you choose the pagination route.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In @.claude/commands/review-pr.md:
- Around line 38-43: The GraphQL sample used by the gh api graphql command is
missing the cursor variable and the after argument for pagination; update the
query to declare a $cursor variable (e.g., $cursor:String) in the query
variables list and pass it into the reviewThreads field as after: $cursor so
reviewThreads(page...) becomes reviewThreads(first:100, after: $cursor),
ensuring the pagination guidance that references after: $cursor will work when
invoking gh api graphql with a cursor value.

---

Nitpick comments:
In @.claude/commands/review-pr.md:
- Around line 51-52: The nested GraphQL field comments(first:30) lacks
pagination and can miss older comments used by your deduplication logic; either
extend that nested selection to include pageInfo (endCursor, hasNextPage) and
accept an after variable (e.g., comments(first:30, after:$commentsAfter)) so
callers can page through comments to match the parent reviewThreads pagination,
or add an explicit inline comment next to comments(first:30) documenting that
the 30-comment cap is intentional and describing the deduplication risk; update
any calling code that relies on the comments selection (the deduplication logic
referenced in the review) to use the paginated cursors if you choose the
pagination route.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: aca6d7ef-8f69-458b-8aa6-c47893856021

📥 Commits

Reviewing files that changed from the base of the PR and between 0a10ec5 and 9c41bdd.

📒 Files selected for processing (1)
  • .claude/commands/review-pr.md

Comment thread .claude/commands/review-pr.md
@jaco-brink jaco-brink changed the title feat: NextSteps-specific PR review skill (NES-1498) feat: nextsteps-specific PR review skill (NES-1498) Mar 31, 2026
The reviewThreads query now declares $cursor:String and passes
after:$cursor so the pagination guidance actually works when copied.

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

jaco-brink commented Mar 31, 2026

Review feedback addressed — round 2 (f712c64)

Fixed:

  • review-pr.md:43 + SKILL.md:46 — Added $cursor:String variable and after:$cursor to GraphQL query so pagination guidance is copy-pasteable

Already handled (round 1):

  • apis/AGENTS.md:180 (CodeRabbit guardrail) — challenged; docs are correct per source verification
  • apis/AGENTS.md:173 (jaco-brink verification) — challenged; confirmed accurate

@github-actions github-actions Bot temporarily deployed to Preview - journeys March 31, 2026 04:33 Inactive
@github-actions github-actions Bot temporarily deployed to Preview - journeys-admin March 31, 2026 04:33 Inactive
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🧹 Nitpick comments (2)
.cursor/skills/review-pr/SKILL.md (1)

67-67: Clarify the initial GraphQL invocation without a cursor parameter.

The query now declares $cursor:String (nullable), but the example command on line 67 doesn't show how to invoke it initially. While the nullable parameter allows omitting -f cursor=... on the first call, implementers may be uncertain.

Suggested clarification
-  }' -f owner=OWNER -f repo=REPO -F pr=NUMBER
+  }' -f owner=OWNER -f repo=REPO -F pr=NUMBER -f cursor=null

Or add a note after line 67:

(Omit `-f cursor=...` on the first call; `$cursor` defaults to `null`.)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.cursor/skills/review-pr/SKILL.md at line 67, Add a short clarifying note
explaining how to invoke the GraphQL query when $cursor is omitted: state that
the query declares $cursor:String (nullable) so on the initial call you should
omit the -f cursor=... argument and the parameter will default to null; update
the documentation near the query example (referencing the $cursor variable and
the query declaration) with a one-line note like "(Omit -f cursor=... on the
first call; $cursor defaults to null.)" so implementers know the initial
invocation behavior.
.claude/commands/review-pr.md (1)

67-67: Clarify the initial GraphQL invocation without a cursor parameter.

The query declares $cursor:String (nullable), but the example command doesn't show the initial invocation syntax. While omitting the cursor parameter works for the first call, adding guidance prevents ambiguity.

Suggested clarification
-  }' -f owner=OWNER -f repo=REPO -F pr=NUMBER
+  }' -f owner=OWNER -f repo=REPO -F pr=NUMBER -f cursor=null

Or add a note:

(On the first call, omit `-f cursor=...` or pass `-f cursor=null`.)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.claude/commands/review-pr.md at line 67, The example command for invoking
the GraphQL query is ambiguous about the nullable $cursor:String parameter;
update the documentation around the example (the curl/graphql invocation shown
ending with }' -f owner=OWNER -f repo=REPO -F pr=NUMBER) to explicitly state how
to call the first request: either omit -f cursor=... entirely or pass -f
cursor=null, or show an explicit initial example without the cursor parameter,
and add the suggested parenthetical note "(On the first call, omit `-f
cursor=...` or pass `-f cursor=null`)."
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In @.cursor/skills/review-pr/SKILL.md:
- Line 191: Update the "If `auto=true`: Always use `COMMENT` regardless of
findings." explanation to mirror the behavior described in the other doc by
appending the operator-review context (e.g., "The operator reviews the PR after
the autonomous cycle completes — they decide whether to request changes or
approve.") so it clearly explains why `COMMENT` is always used; locate the text
by searching for the literal "If `auto=true`" and the `COMMENT` keyword and
replace or extend that sentence to include the operator review sentence
verbatim.
- Line 16: Update the `auto` parameter description so it matches the one in the
other doc: keep "`auto` — `true` or `false` (default `false`). When `true`, skip
the operator confirmation gate (Step 4) and proceed directly to
posting/displaying. Use `COMMENT` as the review event (never `REQUEST_CHANGES`
or `APPROVE` without a human)." and append the phrase "Intended for build-loop
autonomous invocation." to the end of that sentence; locate the `auto` parameter
description in SKILL.md and replace the existing sentence with this synced
version.

---

Nitpick comments:
In @.claude/commands/review-pr.md:
- Line 67: The example command for invoking the GraphQL query is ambiguous about
the nullable $cursor:String parameter; update the documentation around the
example (the curl/graphql invocation shown ending with }' -f owner=OWNER -f
repo=REPO -F pr=NUMBER) to explicitly state how to call the first request:
either omit -f cursor=... entirely or pass -f cursor=null, or show an explicit
initial example without the cursor parameter, and add the suggested
parenthetical note "(On the first call, omit `-f cursor=...` or pass `-f
cursor=null`)."

In @.cursor/skills/review-pr/SKILL.md:
- Line 67: Add a short clarifying note explaining how to invoke the GraphQL
query when $cursor is omitted: state that the query declares $cursor:String
(nullable) so on the initial call you should omit the -f cursor=... argument and
the parameter will default to null; update the documentation near the query
example (referencing the $cursor variable and the query declaration) with a
one-line note like "(Omit -f cursor=... on the first call; $cursor defaults to
null.)" so implementers know the initial invocation behavior.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 5296e3b1-8605-484f-a908-980a3a33163a

📥 Commits

Reviewing files that changed from the base of the PR and between 9c41bdd and f712c64.

📒 Files selected for processing (2)
  • .claude/commands/review-pr.md
  • .cursor/skills/review-pr/SKILL.md

Comment thread .cursor/skills/review-pr/SKILL.md
Comment thread .cursor/skills/review-pr/SKILL.md Outdated
- Sync auto=true explanation in SKILL.md with commands/review-pr.md
- Remove "Intended for build-loop autonomous invocation" — core repo
  is not aware of the build-loop; auto=true is a generic concept.

Addresses CodeRabbit review feedback on PR #8936.
@jaco-brink
Copy link
Copy Markdown
Collaborator Author

Review feedback addressed (6b65f51)

Fixed:

  • .cursor/skills/review-pr/SKILL.md:16 — Synced auto parameter description with .claude/commands/review-pr.md
  • .cursor/skills/review-pr/SKILL.md:191 — Synced auto=true explanation with .claude/commands/review-pr.md
  • Both files: Removed "Intended for build-loop autonomous invocation" — core repo should not reference the workspace build-loop; auto=true is a generic automation concept

Skipped (already resolved by author):

  • apis/AGENTS.md:180 — CodeRabbit false positive on guardrail criteria; verified against both implementations and acknowledged by CodeRabbit
  • apis/AGENTS.md:173 — Self-review comment; verified AGENTS.md criteria matches source implementations

@github-actions github-actions Bot requested a deployment to Preview - journeys April 1, 2026 04:54 Pending
@github-actions github-actions Bot requested a deployment to Preview - journeys-admin April 1, 2026 04:54 Pending
@github-actions github-actions Bot temporarily deployed to Preview - journeys April 1, 2026 04:57 Inactive
@github-actions github-actions Bot temporarily deployed to Preview - journeys-admin April 1, 2026 04:57 Inactive
@jaco-brink jaco-brink merged commit 1ebd921 into main Apr 1, 2026
22 checks passed
@jaco-brink jaco-brink deleted the jacobusbrink/nes-1498-nextsteps-specific-pr-review-skill branch April 1, 2026 05:08
tanflem pushed a commit that referenced this pull request Apr 3, 2026
Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant