signoz-website-frontend-pr-review · git:20260324.6445964 · 2026-03-24 · sha256 e119de352c6916fd

signoz-website-frontend-pr-review git:20260324.6445964A

Immutable. This exact content never changes and is served at /api/v1/blob/e119de352c6916fd.

---
name: signoz-website-frontend-pr-review
description: Review SigNoz frontend pull requests for duplication, architecture, App Router best practices, performance, maintainability, and accessibility. Use when asked to review JS/TS/React/Next.js changes, check components or hooks, evaluate frontend code quality, or review any PR whose changed files are under app/, components/, hooks/, utils/, or similar frontend paths.
---

# SigNoz Frontend PR Review

Review frontend pull requests with a strict, actionable, high-impact-first approach.

## Scope

Apply this skill when changed files include frontend code such as:

- `**/*.js`, `**/*.jsx`, `**/*.ts`, `**/*.tsx`
- `app/**`, `components/**`, `layouts/**`, `hooks/**`, `utils/**`, `constants/**`
- related config files when they affect frontend behavior

If a PR includes docs too, use this skill for code review only.

## Source of Truth

This skill file defines the review rubric (the 13 categories below). Project-specific code conventions and verification commands live in `contributing/site-code.md` — read it before reviewing so you apply the canonical rules, not stale assumptions.

## Review Process

1. Get PR context and changed files.
2. **Read `contributing/site-code.md` in full before starting the review.** It contains the project's icon policy, UI primitive expectations, componentItems data placement rules, async/DOM safety rules, MDX rendering constraints, dependency policy, and required verification commands. Reviewing without reading it first leads to missed project-specific findings.
3. Scan for high-impact issues first (duplication, architecture, performance).
4. Evaluate against the categories below.
5. Leave inline comments for specific issues only.
6. Post exactly one concise summary grouped by severity.

## Review Categories

### 1) DRY and duplication

- Find duplicated logic and repeated patterns.
- Check copy-paste blocks across files.
- Flag code that should be extracted into shared utilities/hooks/components.
- Verify no duplication of existing helpers in `utils/`, `hooks/`, `app/lib/utils.ts`.
- Verify no duplication of reusable UI in `shared/components/` or `components/ui/`.

### 2) Component design and architecture

- Check decomposition and single responsibility.
- Flag prop drilling where composition/context is more suitable.
- Validate correct server vs client component boundaries.
- Check component placement by purpose:
  - shared/reusable -> `shared/components/` or `components/ui/`
  - page-specific -> route-local or co-located
  - feature-specific -> feature folders
- For shared components, verify pattern: `ComponentName.types.ts`, `ComponentName.view.tsx`, `index.tsx` where applicable.

### 3) Next.js 14 App Router practices

- Validate App Router patterns (not Pages Router patterns).
- Check data-fetching boundaries (server components, server actions, route handlers).
- Verify usage of:
  - `next/image`
  - metadata API (`metadata` / `generateMetadata`)
  - `next/dynamic` when needed
  - `next/link` for navigation
  - `next/navigation` hooks (not `next/router`)
- Check presence/usage of route-level loading/error handling where relevant (`loading.tsx`, `error.tsx`, suspense boundaries).

### 4) React 18 practices

- Hooks correctness (`useEffect` deps, cleanup, memoization where needed).
- List key correctness.
- Avoid unnecessary re-renders.
- State management quality and anti-patterns (no direct mutation).

### 5) TypeScript quality

- Minimize/justify `any`.
- Validate type safety and interfaces.
- Check missing return types where clarity matters.
- Check type imports/exports and generic usage.
- Flag avoidable unsafe assertions.

### 6) Performance

- Spot expensive render-path work and memoization gaps.
- Flag likely N+1 fetch patterns.
- Check code-splitting/lazy loading opportunities.
- Verify unoptimized image/asset handling.
- Prefer server components to reduce client bundle when possible.

### 7) Maintainability

- Flag overly complex functions/components.
- Check naming quality.
- Replace magic numbers/strings with constants where appropriate.
- Check logging/error messages quality.
- Flag stale TODO/FIXME items when they introduce risk.

### 8) Folder structure and organization

- Validate placement under `app/`, `components/`, `shared/components/`, `layouts/`, `hooks/`, `utils/`, `constants/`.
- Ensure separation of concerns and consistent naming.
- Flag misplaced/orphaned files.
- Check proper path alias usage.

### 9) Styling and CSS

- Check Tailwind consistency and duplication.
- Treat avoidable `.module.css` additions/expansions as findings when equivalent Tailwind classes/utilities can be used.
- Flag new CSS custom properties, font-family declarations, and visual tokens added in component CSS modules unless there is a clear technical need (for example: pseudo-elements, complex keyframes, or browser-specific behavior Tailwind cannot express cleanly).
- For styling-only PRs, explicitly verify that new visual changes are primarily implemented via existing Tailwind tokens/utilities.
- Flag avoidable inline styles.
- Check responsive utilities and token consistency where applicable.

### 10) Dependencies and imports

- Remove unused imports.
- Check import order/organization.
- Flag circular dependencies.
- Avoid duplicate functionality from existing deps.
- Ensure new deps are justified (per `contributing/site-code.md`).

### 11) Project-specific rules (`contributing/site-code.md`)

- Apply the project-specific rules from `contributing/site-code.md`.
- Pay extra attention to icon usage, existing UI primitives, `constants/componentItems*.ts` data placement, async handler safety, MDX rendering compatibility, and dependency justification.
- Treat hardcoded `slice()` boundaries for logical sub-sections as a `High` finding.

### 12) Error handling and edge cases

- Verify async error handling (`try/catch` where needed).
- Check null/undefined handling.
- Validate loading/error state handling.
- Look for race conditions.

### 13) Accessibility (A11Y)

- Validate ARIA labels and semantic HTML.
- Check keyboard navigation/focus behavior.
- Check contrast and screen-reader compatibility basics.

## Commenting Rules

- Comment only on actual issues; no praise-only comments.
- Prioritize high-impact issues first.
- Be specific: file path, line context, function/component names.
- Suggest concrete fixes (include concise code suggestions when helpful).
- Group related issues when appropriate.
- If no issues in a category, do not mention that category.

## Output Format

### Inline findings

For each finding include:

- short title
- impact
- exact suggested fix
- file reference
- severity (`Critical`, `High`, `Medium`, `Low`)

### One summary comment

Post exactly one concise summary that:

1. Separates Documentation findings from Code findings (if docs were also reviewed)
2. Groups code findings by severity (`Critical`, `High`, `Medium`, `Low`)
3. Highlights highest-priority fixes
4. References inline comments
5. Stays concise and actionable

## Suggested Commands

```bash
# PR context
gh pr view <PR_NUMBER>
gh pr diff <PR_NUMBER>

# project-specific standards
cat contributing/site-code.md

# find similar code
find components shared -name "*.tsx" -type f
rg -n "<pattern>" utils hooks app/lib components shared

# inspect imports and usage
rg -n "^import " app components hooks utils shared
```

## Guardrails

- Prefer existing patterns over introducing new abstractions by default.
- For style changes, prefer Tailwind-first implementations and avoid introducing new component-level CSS systems when existing Tailwind patterns already solve the requirement.
- Avoid speculative refactors outside PR scope unless there is clear risk reduction.
- Keep feedback decision-oriented and implementable without ambiguity.
- Read `contributing/site-code.md` during the review instead of relying on memory or partial summaries.