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.