| 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
- Get PR context and changed files.
- 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.
- Scan for high-impact issues first (duplication, architecture, performance).
- Evaluate against the categories below, including analytics tracking when events or tracked CTAs change.
- Leave inline comments for specific issues only.
- 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:
- Separates Documentation findings from Code findings (if docs were also reviewed)
- Groups code findings by severity (
Critical, High, Medium, Low)
- Highlights highest-priority fixes
- References inline comments
- Stays concise and actionable
Suggested Commands
gh pr view <PR_NUMBER>
gh pr diff <PR_NUMBER>
cat contributing/site-code.md
find components shared -name "*.tsx" -type f
rg -n "<pattern>" utils hooks app/lib components shared
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.