d2c_
View sourcecached

Design-System PR Review

You are reviewing a GitHub pull request against this project's design system as captured in .claude/d2c/design-tokens.json. Your job is to find violations the PR introduces — hardcoded values that should be tokens, new markup that should reuse an existing component, imports of the wrong library, convention drift, and a11y misses — and either print a structured report or post them as inline PR review comments. You do NOT review the whole codebase; that is /d2c-audit's job. You review only what the PR changed.

Non-negotiables

These rules hold across every phase of this skill. No exceptions.

  1. Design tokens MUST be loaded before any decision. Read .claude/d2c/design-tokens.json. If it is missing, unreadable, or has d2c_schema_version < 1, STOP AND ASK the user to run /d2c-init (or /d2c-init --force if outdated).
  2. NEVER use a library outside preferred_libraries.<category>.selected. The user explicitly chose which library to use for each capability. NEVER substitute an installed-but-not-selected library. If the design requires a capability not covered by preferred_libraries, STOP AND ASK.
  3. NEVER hardcode color, spacing, typography, shadow, or radius values. Every visual value MUST reference a design token from design-tokens.json. No raw hex, no magic numbers, no exceptions.
  4. MUST reuse existing components when an existing component can serve the need. Check the components array in design-tokens.json before creating anything new. If an existing component can do the job, MUST use it.
  5. MUST follow project conventions when confidence > 0.6 and value ≠ "mixed". Project conventions (declaration style, export style, type definitions, import ordering, file naming, CSS wrapper, barrel exports, props pattern) override framework defaults.
  6. NEVER re-decide a locked component or token. Read decisions.lock.json from the IR run directory at the start of every phase after Phase 2. Only nodes with status: "failed" may have their component choice or token mapping changed. If a locked decision must change, STOP AND ASK.

When any rule is ambiguous, STOP AND ASK — do not guess.

Note for d2c-pr-review: These are the rules whose violations this skill reports. Every finding is a violation of one of the rules above (or a convention/a11y derivative). This skill is read-only on the project source — it never modifies files in the working tree. Optional GitHub side effects are limited to posting review comments via gh when --comment is passed.

Arguments

Parse $ARGUMENTS:

  • <PR-number-or-URL> (required, positional) — a PR number (1234), a gh-style ref (#1234), or a full URL (https://github.com/<owner>/<repo>/pull/1234). The skill resolves the owner/repo/number triple via gh pr view. STOP AND ASK when the value cannot be parsed into any of these forms.
  • --comment (optional) — post findings as inline GitHub PR review comments via gh. Without this flag, the skill prints a Markdown report to stdout and does not touch GitHub. With this flag, ALL findings post atomically as a single review (one notification, not one per finding).
  • --severity warning|error (optional, default warning) — gates the GitHub review event field. warning → event: COMMENT (advisory). error → event: REQUEST_CHANGES when at least one error-severity finding fires (see _shared/rules.md). Without --comment, this flag is informational only — it appears in the report header so the user knows what the GitHub submission would have looked like.

Unknown flags → emit a one-line warning and continue. Never STOP AND ASK on unknown flags — keeps the skill scriptable.

Pre-flight Check

Before any phase:

  1. Check that .claude/d2c/design-tokens.json exists. If it does not, automatically run /d2c-init to scan the codebase and generate tokens. Wait for d2c-init to complete successfully before continuing. If d2c-init fails, STOP AND ASK — do not proceed with a review that cannot reference design tokens.
  2. Schema version check. Read d2c_schema_version from design-tokens.json. If it is missing or less than 1, STOP AND ASK: "design-tokens.json uses schema version {version or 'none'} but the current version is 1. Run /d2c-init --force to regenerate before this review can produce reliable results."
  3. Verify gh is installed and authenticated. Run gh auth status (silently). If it fails, STOP AND ASK with the exact gh auth login command the user needs to run. Do NOT attempt to authenticate.

Phase 1 — Resolve the PR

  1. Parse the <PR-number-or-URL> argument into a ref gh accepts. URL forms must extract owner/repo so the report header can echo them.
  2. Run gh pr view <ref> --json number,headRefName,baseRefName,url,files,author,title. Capture the JSON output.
  3. Verify the response includes a non-empty files[]. If empty, the PR has no changes worth reviewing — emit a short report ("PR has no file changes") and exit cleanly.
  4. Store headRefName, baseRefName, and the PR URL for later use in the report and comment payloads.

When gh pr view returns a non-zero exit code, surface the exact stderr to the user with a one-line summary. Do NOT retry — the failure is almost always auth or a typo, not transient.

Phase 2 — Fetch and parse the diff

  1. Fetch the unified diff: gh pr diff <ref> (stdout).
  2. Pipe it into the diff parser: 
    gh pr diff <ref> | node skills/d2c-pr-review/scripts/parse-diff.js -
    Resolve the script path with the same Glob fallback chain other skills use (references/-relative first, then ~/.agents/, ~/.claude/, then a global Glob for **/parse-diff.js).
  3. The parser returns an array of { file, binary, renamed_from, added_lines }. The skill only scans added_lines — never context, never removed lines. Binary files are reported in the header summary only ("3 binary files skipped") and never produce findings.
  4. File-extension filter. Read the framework field from design-tokens.json and apply the same extension list d2c-audit uses (§Audit 1) — .tsx/.jsx/.ts/.js for React/Solid/Qwik, .vue for Vue, .svelte for Svelte, .component.* for Angular, .astro for Astro, plus .css/.scss/.module.css/.module.scss regardless of framework. Files outside this set are listed in the header summary ("12 files outside framework scope: skipped") and skipped.
  5. Path filter. Skip files matching: node_modules/, .next/, dist/, build/, coverage/, *.test.*, *.spec.*, __tests__/, and config files (*.config.*, postcss.*, tailwind.config.*). Same exclusions as d2c-audit.

Phase 3 — Run rule checks

Run each check below against the surviving files' added_lines. Every finding emits an object with:

{
  "rule_id": "PR-TOKEN-COLOR",
  "severity": "error",
  "file": "src/components/Header.tsx",
  "line": 42,
  "message": "Hardcoded color #3B82F6 matches the `primary` token.",
  "suggested_fix": "Use the token: bg-primary"
}

The full rule list and severity defaults live in _shared/rules.md. Resolve the catalog with the same Glob fallback the script paths use.

3.1 — Token drift checks

For each added_lines[i].content, regex-scan for:

  • Colors — #[0-9a-fA-F]{3,8}, rgb(, rgba(, hsl(, hsla(. Convert each match to lowercase 6-digit hex. If it exactly matches a value in design-tokens.json.colors, emit PR-TOKEN-COLOR with the matching token name. If no match, emit PR-TOKEN-COLOR-UNDOC (warning).
  • Spacing — match (padding|margin|gap|width|height):\s*\d+(px|rem|em) and Tailwind arbitrary brackets [pm]-\[\d+(px|rem|em)\], gap-\[…\], [wh]-\[…\]. If the resolved value matches a design-tokens.json.spacing entry, emit PR-TOKEN-SPACING.
  • Typography — match font-size:\s*\d+(px|rem), text-\[\d+(px|rem)\], font-weight:\s*\d{3}, font-\[\d{3}\]. If matches a token, emit PR-TOKEN-TYPOGRAPHY.
  • Shadow — box-shadow:\s*[^;]+ matching a shadows token → PR-TOKEN-SHADOW.
  • Border radius — border-radius:\s*\d+(px|rem) matching a borders token → PR-TOKEN-BORDER.

Conversion rules and the "exact match only" policy mirror d2c-audit/SKILL.md §Audit 1 — do not loosen them.

3.2 — Component reuse checks

Scan added_lines for raw HTML elements per framework (same set as d2c-audit §Audit 3): <button, <input, <select, <textarea, <dialog, <table. For each match, check components[] in design-tokens.json for a name containing the element keyword (case-insensitive). When found, emit PR-COMPONENT-REUSE with suggested_fix: "Reuse <ComponentName> from <source-path>".

Skip this check entirely for files INSIDE component directories (the implementation files themselves). Use the framework's component directory list — for React/Next that's src/components/, components/; for Vue src/components/; for Svelte src/lib/components/; for Angular src/app/*/components/; for Astro src/components/. Hardcode this list in the skill — it tracks the same convention d2c-init uses.

3.3 — Library policy checks

Build a lookup from preferred_libraries:

  • For each category in preferred_libraries, take its selected library and list any other libraries known to provide the same capability (the installed array contains the full set; everything in installed that is not selected is a violator candidate).

For each added_lines[i].content starting with import (or matching require('<pkg>')), extract the package name. If it appears in any category's violator list, emit PR-LIBRARY-WRONG with suggested_fix: "Replace import with: import … from '<selected>'". The exact import snippet should mirror the existing project's usage — read one file that already imports the selected library and copy its pattern.

For data-fetching specifically, also flag raw fetch( and useEffect(+useState( combinations in component bodies when preferred_libraries.data_fetching.selected is a higher-level library (React Query, SWR, Nuxt's useFetch, etc.). Emit PR-LIBRARY-RAW-FETCH.

3.4 — Convention drift checks

Load the conventions section. For each convention where confidence > 0.6 (or override: true) and value !== "mixed", run the matching check below against the added_lines of the file:

  • component_declaration — first added function definition's style mismatches the declared style → PR-CONVENTION-DECL.
  • export_style — added export lines mismatch → PR-CONVENTION-EXPORT.
  • type_definition — added interface ... when value is "type" (or vice versa) → PR-CONVENTION-TYPE-DEF.
  • type_location — added type/interface definitions inside the component file when value is "separate_file" (or vice versa) → PR-CONVENTION-TYPE-LOC.
  • file_naming — new file's basename mismatches (PascalCase / kebab-case / camelCase) → PR-CONVENTION-FILE-NAME. Only applies to files that did not exist on baseRefName (use gh api repos/{owner}/{repo}/contents/{path}?ref=<base> to test existence — a 404 means new file).
  • import_ordering — added imports violate the documented group order → PR-CONVENTION-IMPORT-ORDER.
  • css_utility_pattern — added Tailwind class strings not wrapped by the project's utility function (e.g., cn(...) from @/lib/utils) → PR-CONVENTION-CSS-UTIL.
  • barrel_exports — new component file in a directory whose index.ts was not updated → PR-CONVENTION-BARREL.
  • props_pattern — added function signature does not match destructured / object-style preference → PR-CONVENTION-PROPS.

test_location is informational only in d2c-init; skip it here too.

3.5 — A11y checks

Same regexes as d2c-audit §Audit 5, scoped to added lines only:

  • <img without alt= → PR-A11Y-ALT.
  • <button / <a with no text content and no aria-label → PR-A11Y-LABEL.
  • Heading levels skip a level within the same file's added_lines (e.g., new <h1> followed by new <h3> with no intervening <h2> in the post-image of the file — read the full post-image when in doubt) → PR-A11Y-HEADING-SKIP.
  • onClick / @click / (click) / onclick on <div / <span / <p without role="button" and without tabIndex → PR-A11Y-CLICK-DIV.
  • <input / <select / <textarea without <label> association and without aria-label / aria-labelledby → **PR-A11Y-INPUT-LABEL`.

Phase 4 — Resolve Jira (optional)

Run the protocol in _shared/jira.md. The PR-review skill uses Jira context for header cross-reference only — Phase 3 does NOT consume the ticket brief. When a key resolves, render it in the report header as Jira: PROJ-123. When nothing resolves, render Jira: —. NEVER STOP AND ASK because Jira is unavailable.

Phase 5 — Render the report (default mode)

When --comment is NOT passed, print a Markdown report to stdout. Format:

# d2c PR review — <repo-owner>/<repo-name>#<PR-number>

**Title:** <PR title>
**Author:** @<author-login>
**Base ↔ Head:** <baseRefName> ↔ <headRefName>
**URL:** <PR URL>
**Jira:** <PROJ-123 or —>
**Severity gate:** <warning|error> (would post as `<COMMENT|REQUEST_CHANGES>`)

## Summary

| Severity | Count |
|---|---|
| error | <n> |
| warning | <n> |
| Total | <n> |

Scope: <files-reviewed> files, <added-lines-total> added lines, <binary-skipped> binary files skipped, <out-of-framework-skipped> non-framework files skipped.

## Findings

### src/components/Header.tsx

| Line | Rule | Severity | Finding | Suggested fix |
|---|---|---|---|---|
| 42 | PR-TOKEN-COLOR | error | Hardcoded color `#3B82F6` matches the `primary` token. | Use `bg-primary` |
| 87 | PR-A11Y-ALT | error | `<img>` added without `alt` attribute. | Add `alt=""` (decorative) or descriptive text |

### src/app/dashboard/page.tsx

| Line | Rule | Severity | Finding | Suggested fix |
|---|---|---|---|---|
| 23 | PR-LIBRARY-WRONG | error | Imports `axios` but `data_fetching.selected` is `@tanstack/react-query`. | Replace with `import { useQuery } from '@tanstack/react-query'` |

## Files with no findings (clean against the rule set)

- src/components/Footer.tsx
- src/app/dashboard/loading.tsx

When no findings fire, replace the "Findings" section with a single line: No design-system violations detected in the changed lines. — keep the header so the user sees what was reviewed.

Phase 6 — Post review comments (--comment mode)

Submit ALL findings as a single GitHub review so the user receives one notification rather than one per finding.

Steps:

  1. Build a comments array. Each entry: 
    {
  "path": "src/components/Header.tsx",
  "line": 42,
  "side": "RIGHT",
  "body": "**[PR-TOKEN-COLOR · error]** Hardcoded color `#3B82F6` matches the `primary` token.\n\nSuggested fix: use `bg-primary`."
}
  2. Decide the review event: REQUEST_CHANGES when --severity error AND at least one finding has severity: "error"; otherwise COMMENT.
  3. Submit the review with one gh api call: 
    gh api repos/{owner}/{repo}/pulls/{number}/reviews \
  -X POST \
  -F event="<COMMENT|REQUEST_CHANGES>" \
  -F body="d2c PR review — <n> finding(s) against the project design system." \
  --input <(jq -n --argjson comments "$COMMENTS_JSON" '{event: $ENV.EVENT, body: $ENV.BODY, comments: $comments}')
    Construct the payload in a temp file when jq-piped stdin is awkward in the host shell. The skill does NOT depend on jq being installed in the user's project — fall back to writing the JSON body to a temp file and gh api ... --input <file>.
  4. Capture the returned review URL. Print one line to the user: Posted review with <n> finding(s): <url>.
  5. On any gh api failure, do NOT retry. Print the stderr to the user with one line of context: "Review submission failed — findings have NOT been posted." Then print the Markdown report to stdout so the work isn't lost.

The skill MUST NOT post comments on any line outside added_lines — GitHub will reject those with 422 anyway. The diff parser's line numbers are post-image right-side lines, which is exactly what the GitHub Pulls API expects when side: "RIGHT".

NEVER post a comment that contains the full ticket brief from _shared/jira.md — Jira context stays in the header line, never in inline comments. Customer names and internal-only ticket content MUST NOT leak into a PR review.

Critical Reminders

  • Scope is the diff, not the codebase. For whole-project audits, redirect the user to /d2c-audit.
  • Read-only on the working tree. The skill MUST NOT modify project files. Optional GitHub side effects only happen with --comment.
  • One review submission, not many. All findings post atomically.
  • Jira is optional. Never fail because the Atlassian MCP is missing or a ticket can't be loaded.
  • Mirror d2c-audit's rule definitions. When the same check appears in both skills, the implementations MUST match — drift between the two confuses users.