signoz-website-frontend-pr-review

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, listicle/discovery 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, including analytics tracking when events or tracked CTAs change.
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) Analytics and event tracking

• Review every new or changed tracked interaction that uses TrackingLink, TrackingButton, useLogEvent, HubSpot form tracking, signup tracking, or manual Website Click events.
• Use the common event names already present in the website unless there is a strong reason to introduce a new event:
  • Website Click
  • Website Page View
  • User Signed Up
  • HubSpot Form Submitted
• For Website Click, keep clickName reusable and query-friendly. It should describe the stable action/object, not the page, section, clicked item label, or campaign context.
  • Good: Sign Up Button, Contact Us Button, Docs Link, Comparison Link, Migration Link, Search Icon Click
  • Bad: Agent Native Page Hero Get Started - Free, Migrate from Datadog Migration Link
• Put distinctions in attributes intended for breakdowns:
  • clickText: exact visible label or item text, for example Get Started - Free, Migrate from Datadog, Read Documentation
  • clickLocation: placement/context, for example Top Navbar, Hero Section, Teams Pricing Card, Agent Native Observability Page Hero
  • pageLocation: current route, for example /, /teams, /contact-us
• For repeated menu or list items, sibling items should usually share the same clickName; use clickText to distinguish the clicked item.
  • Good: clickName = "Migration Link", clickText = "Migrate from Datadog", clickLocation = "Top Navbar"
  • Bad: clickName = "Migrate from Datadog Migration Link"
• Avoid duplicating context across attributes. If the page or section is already captured by clickLocation and pageLocation, do not repeat it in clickName.
• Reuse existing names before inventing new ones. Search for similar clickName values such as Sign Up Button, Contact Us Button, Docs Link, and Search Icon Click before approving new tracking.
• Avoid PII in click events. Do not add email, free-form descriptions, or other personal data to Website Click attributes unless there is a clear existing product requirement.
• For conversion funnels, verify the chosen properties make common Mixpanel questions easy: count clicks by action, break down by item text/location, and funnel click -> page view -> signup/form submit.

9) 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.

10) Styling and CSS

• Check Tailwind consistency and duplication.
• Prefer existing Tailwind utilities and SigNoz design tokens over arbitrary values wherever the visual impact is negligible.
• Flag avoidable arbitrary Tailwind values such as text-[13px], rounded-[17px], or hardcoded hex colors when a close built-in utility or existing token would preserve the design.
• Allow arbitrary values when they are genuinely layout-specific, animation-specific, or intentionally brand/asset-matching, but ask the author to document or justify the exception in the PR when it is not obvious.
• 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.

11) 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).

12) 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/listicles/*.json data placement, async handler safety, MDX rendering compatibility, and dependency justification.
• Treat hardcoded slice() boundaries for logical sub-sections as a High finding.

13) 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.

14) 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

# 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.