Skip to content

feat: DLT-3049 add code review rules, skill, agent, and command#1154

Merged
belumontoya merged 2 commits intostagingfrom
feat/DLT-3049-code-review-path-rules
Mar 27, 2026
Merged

feat: DLT-3049 add code review rules, skill, agent, and command#1154
belumontoya merged 2 commits intostagingfrom
feat/DLT-3049-code-review-path-rules

Conversation

@belumontoya
Copy link
Copy Markdown
Collaborator

Summary

  • Add .claude/rules/code-review.md — path-scoped rules file with 9 categories (25 rules) from Brad's DLT-3049 checklist. Auto-loads when editing files in dialtone-vue, dialtone-css, dialtone-tokens, dialtone-icons.
  • Add .claude/skills/review.md — review logic skill that reads the rules file, detects scope from git diff, maps files to rule categories, and outputs findings. Delegates to a background agent for diffs with 10+ files.
  • Add .claude/agents/review.md — background agent fallback for large reviews. Runs in isolated context to protect the main conversation.
  • Add .claude/commands/review.md/review slash command trigger.

Ticket

https://dialpad.atlassian.net/browse/DLT-3049

Test plan

  • Run /review with staged changes on a few .vue files — verify findings reference the correct rule categories
  • Run /review accessibility — verify only accessibility rules are applied
  • Edit a file in packages/dialtone-vue/ — verify code-review.md rules load alongside vue-components.md
  • Verify /review appears in skill listing

Add Dialtone code review tooling based on the review checklist
defined in DLT-3049. Includes path-scoped rules (9 categories,
25 rules), a /review skill with agent fallback for large diffs,
and a slash command trigger.
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 87501e4a9e

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "Codex (@codex) review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "Codex (@codex) address that feedback".

Comment thread .claude/agents/review.md Outdated
For each file:

1. Read the full file for context
2. Read the diff for that file: `git diff -- <filepath>`
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Include staged hunks in per-file review diff

This step uses git diff -- <filepath>, which only shows unstaged working-tree changes; it omits staged changes (and can be empty when the review scope came from staged/base-branch diffs). In the 10+ file path where this agent is used, that can produce false "clean" results because the agent never sees the actual modified hunks it is supposed to evaluate.

Useful? React with 👍 / 👎.

Comment thread .claude/skills/review.md Outdated
Comment on lines +26 to +27
2. If there are staged changes → `git diff --cached --name-only`
3. If there are unstaged changes → `git diff --name-only`
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Merge staged and unstaged file scopes

The scope detection is written as an ordered conditional sequence, so when staged files exist the flow can stop at git diff --cached --name-only and skip unstaged files. That contradicts the documented /review behavior (“all staged and unstaged changes”) and causes partial reviews whenever developers keep some edits unstaged.

Useful? React with 👍 / 👎.

Comment thread .claude/skills/review.md Outdated
|---|---|
| `*.vue` | Reuse & Duplication, Code Quality, Vue Correctness, API & Library Design, i18n & Assets, Accessibility |
| `*.less` | CSS / Styling |
| `*.test.js` | Testing |
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Cover TypeScript test files in rule mapping

The testing category mapping only matches *.test.js, but this repo contains Dialtone tests named *.test.ts (for example under packages/dialtone-tokens/sync-scripts). Those files will miss Testing-rule checks, so reviews for token test changes become systematically incomplete.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Contributor

@iropolo Ignacio Ropolo (iropolo) left a comment

Choose a reason for hiding this comment

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

MORE RULES!!

Image

Copy link
Copy Markdown
Contributor

@braddialpad Brad Paugh (braddialpad) left a comment

Choose a reason for hiding this comment

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

amazing thanks!

- Use git diff HEAD in agent to capture both staged and unstaged hunks
- Merge staged and unstaged file scopes in skill scope detection
- Add *.test.ts to testing category mapping
@belumontoya belumontoya merged commit 6bdef46 into staging Mar 27, 2026
9 checks passed
@belumontoya belumontoya deleted the feat/DLT-3049-code-review-path-rules branch March 27, 2026 14:51
Brad Paugh (braddialpad) pushed a commit that referenced this pull request Mar 31, 2026
# [9.176.0](dialtone/v9.175.2...dialtone/v9.176.0) (2026-03-31)

### Bug Fixes

* **Rich Text Editor:** DLT-3190 replace custom JSON-to-markdown converter with @tiptap/static-renderer ([#1148](#1148)) ([9629c70](9629c70))

### Documentation

* DLT-3151 blog post for ui-kits migration and script ([#1156](#1156)) ([069ad3c](069ad3c))
* **Editor:** DLT-3050 updated editor docs with recent changes ([#1139](#1139)) ([04bfabb](04bfabb))
* **Icons:** DLT-3220 update contributing and add claude rules and skill ([#1151](#1151)) ([296a83d](296a83d))

### Features

* **Dialtone Cli,dialtone Query Core:** DLT-3163 extract query core and add CLI ([#1142](#1142)) ([65090e6](65090e6))
* DLT-3049 add code review rules, skill, agent, and command ([#1154](#1154)) ([6bdef46](6bdef46))
* **Icons:** DLT-3219 align star/sparkle icons with dp brand star ([#1152](#1152)) ([ef39afc](ef39afc))
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.

3 participants