Write skills and agents with quality focus #989
Conversation
…regression Add comprehensive Claude Code configuration focusing on three core principles: 1. Performance - Server Components, query optimization, caching patterns 2. No Regression - Automated verification checks, testing guidelines 3. Maintainability - Code patterns, style guides, accessibility Configuration includes: - CLAUDE.md: Project guidelines and coding standards - Slash commands: /verify, /review, /perf, /pre-commit, /test-coverage - Skills: api-development, component-development, database-changes, testing - Hooks: Post-edit reminders for TypeScript changes
📝 WalkthroughWalkthroughAdds a set of new Claude configuration and guidance documents under Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
Pre-merge checks✅ Passed checks (3 passed)
Comment |
Coverage Report
File CoverageNo changed files found. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI Agents
In @.claude/commands/test-coverage.md:
- Line 76: Remove the stray closing code fence present at the end of the file:
delete the orphan "```" on line 76 so the existing typescript code block (opened
at the typescript fence around line 67 and closed at line 71) remains properly
balanced and no extra closing fence remains.
In @.claude/settings.json:
- Around line 31-41: The Stop hook contains a hardcoded repo path in the command
string which breaks in other environments; update the "command" in the Stop hook
to avoid the absolute path by either removing the `cd
/home/user/programming.in.th &&` so the script runs from the current working
directory, or replace it with a repo-root discovery like `git rev-parse
--show-toplevel` to cd dynamically; keep the rest of the git diff check and the
echo logic intact so the matcher and hook behavior (the "Stop" hook and its
"command") remain unchanged.
In @.claude/skills/component-development/SKILL.md:
- Around line 183-191: The onKeyDown handler on the <button> that checks e.key
=== 'Enter' is unnecessary and incorrect: native button elements already handle
Enter and Space, so remove the onKeyDown prop and rely on the button's onClick
(handleClick); if you intended to make a non-interactive element
keyboard-accessible instead, move the handler to that element (e.g., a <div
role="button">) and handle both 'Enter' and ' ' (Space) along with appropriate
ARIA and focus management.
- Around line 210-216: The Examples in Codebase section lists non-existent files
(remove references to `src/components/TaskCard.tsx`,
`src/components/SubmitModal.tsx`, and `src/components/NavBar.tsx`); update the
paragraph to only reference actual components (e.g., keep
`src/components/Code.tsx`) or replace the invalid entries with other real
components from the repo, ensuring the list matches current filesystem names.
🧹 Nitpick comments (6)
.claude/skills/api-development/SKILL.md (2)
26-34: Consider validating coerced numbers more strictly.
z.coerce.number()can silently convert invalid inputs toNaN. Consider adding refinements to ensure the coerced value is a valid number:🔎 Suggested improvement
const RequestSchema = z.object({ id: z.string().min(1), - limit: z.coerce.number().optional().default(10), + limit: z.coerce.number().positive().int().optional().default(10), filter: z.enum(['all', 'active', 'archived']).optional() })This ensures the limit is a positive integer rather than potentially
NaNor negative.
78-94: Consider creating shared error response utilities.These error response patterns are excellent for consistency, but they're defined as inline examples. Creating actual shared utilities in a file like
src/lib/api/responses.tswould ensure consistency across all API routes and reduce duplication.Do you want me to generate a proposal for a shared error response utility module?
.claude/skills/testing/SKILL.md (1)
83-101: Consider documenting mock cleanup best practices.The mocking examples are correct, but it would be helpful to mention that
vi.clearAllMocks()should typically be called in abeforeEachorafterEachhook to prevent test pollution, especially when usingvi.mock()at the module level.🔎 Suggested addition
Add a note after the mocking examples:
**Note**: When using module-level mocks with `vi.mock()`, ensure you clear or reset mocks between tests: \`\`\`typescript beforeEach(() => { vi.clearAllMocks() // Clear call history // or vi.resetAllMocks() // Clear call history and reset implementations }) \`\`\`.claude/skills/component-development/SKILL.md (1)
147-156: Verify React.memo usage guidance aligns with React 18+ best practices.The
React.memoexample is correct, but note that React 18+ with automatic batching and concurrent features reduces the need for manual memoization in many cases. Consider adding a note about when memo is actually beneficial vs. premature optimization.💡 Suggested clarification
Add a note before the example:
### 2. Use React.memo for Expensive Renders **Use when**: Components with expensive render logic that receive the same props frequently. **Skip when**: Simple components or components that rarely re-render with the same props (premature optimization).CLAUDE.md (1)
1-195: Comprehensive project guidelines document that effectively operationalizes the PR's quality-focused architecture.Overall assessment:
- Scope: Covers architecture, principles, tech stack, commands, patterns, conventions, gotchas, and security in a single reference
- Alignment: The three principles (Performance First, No Regression, Maintainability) directly align with the PR's stated objectives and are reflected in the command workflows added elsewhere in this PR
- Actionability: Checklists, code examples, and specific commands make the guidelines immediately applicable
- Completeness: Addresses key areas for a Next.js 15 + React 19 + Prisma + PostgreSQL platform with competitive programming focus
The integration with other files in this PR (.claude/commands/* for specific workflows, .claude/skills/* for domain expertise) creates a cohesive developer guidance system.
Minor suggestion: Consider adding a brief troubleshooting section for common setup or runtime issues (e.g., Prisma migration workflows, environment variable requirements), though this may be better suited to a separate README.
.claude/skills/database-changes/SKILL.md (1)
94-110: Minor: Clarify cursor pagination semantics in documentation.Line 100's comment "skip the cursor" is slightly ambiguous. The
skip: 1skips the cursor item itself to avoid duplicates on subsequent fetches, which is correct. Consider rephrasing to "skip the cursor item" for clarity.🔎 Proposed clarification
const submissions = await prisma.submission.findMany({ take: 10, - skip: 1, // Skip the cursor + skip: 1, // Skip the cursor item to avoid duplicates cursor: { id: lastSubmissionId }, orderBy: { createdAt: 'desc' } })
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
.claude/commands/perf.md.claude/commands/pre-commit.md.claude/commands/review.md.claude/commands/test-coverage.md.claude/commands/verify.md.claude/settings.json.claude/skills/api-development/SKILL.md.claude/skills/component-development/SKILL.md.claude/skills/database-changes/SKILL.md.claude/skills/testing/SKILL.mdCLAUDE.md
🧰 Additional context used
🪛 LanguageTool
.claude/commands/verify.md
[style] ~51-~51: This phrase is redundant. Consider writing “details”.
Context: ...FAIL** ``` If any check fails, provide specific details about the failures and suggest fixes.
(SPECIFIC_DETAILS)
🪛 markdownlint-cli2 (0.18.1)
.claude/commands/verify.md
39-39: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
.claude/commands/test-coverage.md
76-76: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
.claude/skills/testing/SKILL.md
19-19: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🔇 Additional comments (26)
.claude/skills/api-development/SKILL.md (4)
1-10: LGTM! Clear skill definition.The skill metadata and introduction are well-structured and provide clear context for when this skill should be applied.
53-72: Excellent authentication and authorization pattern.The distinction between authentication (401) and authorization (403) is properly implemented, and the pattern is secure and clear.
98-113: Strong database query example.Demonstrates proper field selection, pagination, and relation handling. This follows Prisma best practices and avoids common performance pitfalls.
133-138: All referenced example files exist. The documentation accurately points to valid API routes in the codebase..claude/commands/verify.md (1)
1-51: LGTM! Well-structured verification workflow.The three-step verification process (types → lint → tests) follows best practices for CI/CD pipelines. The output format is clear and actionable.
Note: The static analysis hints about "specific details" being redundant and the missing language identifier for the output template can be safely ignored—both are appropriate for this documentation context.
.claude/settings.json (2)
1-18: LGTM! Permissions configuration is appropriately scoped.The allowed tools and commands are reasonable for a development workflow. The permissions follow the principle of least privilege while enabling necessary operations.
20-29: PostToolUse hook looks good.The hook correctly detects TypeScript file modifications and provides useful feedback. The file extension check is appropriate.
.claude/skills/testing/SKILL.md (6)
1-33: LGTM! Clear testing structure and stack overview.The testing stack (Vitest, Playwright, Testing Library) is modern and appropriate. The test location guidance with the directory structure example is helpful.
Note: The static analysis hint about the missing language identifier for the directory tree can be ignored—it's a text-based visualization, not code.
35-64: Excellent unit test structure example.The basic test structure follows best practices with proper use of
describe,beforeEach, and clear test cases. Good coverage of valid input, invalid input, and edge cases.
127-173: Strong E2E testing patterns.The Playwright examples demonstrate proper async handling, navigation waiting, and authentication setup. These patterns follow Playwright best practices.
257-266: Excellent "No Regression" checklist.The checklist aligns perfectly with the PR objectives around "No Regression" and provides actionable items for developers.
268-272: All referenced test files exist and documentation is accurate.Verified that the three example test files referenced in the documentation are present in the codebase:
src/lib/scoring/scoring.spec.ts✓src/lib/api/schema/searchParams.spec.ts✓tests/solution.spec.ts✓
103-125: No action needed — @testing-library/jest-dom is already configured.The example code is correct. The
toBeInTheDocument()matcher is already available globally becausevitest.setup.mjsimports@testing-library/jest-dom/vitest. The test code shown will work as written without requiring changes..claude/skills/component-development/SKILL.md (5)
1-17: LGTM! Clear component organization structure.The component location guidance provides a logical structure for organizing React components by feature.
19-66: Excellent Server/Client component guidance.The distinction between Server and Client components is clearly explained with appropriate examples. The guidance to use Server Components by default and only add
'use client'when needed aligns with Next.js 13+ best practices.
84-128: Strong data fetching patterns for both server and client.The examples correctly demonstrate:
- Direct database queries in Server Components
- SWR for client-side data fetching
- Proper loading and error states
158-173: Excellent Next.js Image optimization example.The
Imagecomponent usage demonstrates proper width/height specification and alt text for accessibility.
68-82: No action required. All custom Tailwind color classes referenced in the documentation (prog-primary-500,prog-primary-600,prog-gray-100,prog-gray-500,prog-white-100,prog-white-500) are properly defined intailwind.config.js(lines 14-28)..claude/commands/test-coverage.md (1)
1-76: Excellent structure and comprehensive coverage guidance.The document provides clear, actionable steps for test coverage analysis with well-defined:
- Coverage report generation via
pnpm test:coverage- Per-file analysis for specific targets
- Priority assessment framework (Critical → Low) centered on business impact
- Structured markdown output template for consistent reporting
The tiering system (Auth/scoring > API routes > UI logic > presentation) aligns well with the project's competitive programming focus.
.claude/commands/perf.md (1)
1-72: Well-structured performance analysis workflow with comprehensive coverage.The command document effectively operationalizes the "Performance First" principle from CLAUDE.md through:
- Clear decision framework for Server vs Client Components
- Data fetching optimization (SWR config, revalidation)
- Database-specific checks (selective fields, N+1 detection, indexing)
- Frontend concerns (bundle size, images, lazy loading, memory leaks)
- Structured output format with priority tiers and actionable recommendations
The analysis areas directly support the project's Next.js 15 + React 19 + Prisma stack and complement the performance checklist in CLAUDE.md (lines 168–175).
CLAUDE.md (3)
23-43: Well-articulated core principles with specific, actionable guidance.The three principles (Performance First, No Regression, Maintainability) are clearly defined with concrete directives:
- Performance: Server Components by default, ISR revalidation, SWR caching, query optimization, compression
- No Regression: ALWAYS run
check-types,test, andlintwith clear justification- Maintainability: TypeScript strict mode, Zod validation, component focus
This foundation effectively aligns with the PR objectives and provides a clear decision framework for developers. The emphasis on "ALWAYS" for type checking and testing is particularly strong for preventing regressions in a competitive programming platform where correctness is critical.
72-142: Code patterns section provides clear, real-world examples for the tech stack.The pattern examples effectively demonstrate:
- Server Component default pattern (TasksPage without
'use client')- Client Component usage for state/hooks/browser APIs
- Zod validation in API routes with proper error handling
- SWR configuration with error/loading states
- Prisma best practices (singleton pattern, selective fields, relation handling)
These examples support the core principles and provide developers with immediate reference implementations. The guidance to prefer separate queries over complex relations (line 141) is a good performance guideline.
160-195: Comprehensive coverage of gotchas, checklists, and security guidelines.The document addresses:
- Prisma singleton pattern (critical for avoiding connection pool exhaustion)
- Auth pattern guidance (getServerUser())
- File upload security (presigned S3 URLs, no direct uploads)
- Code compression buffer handling
- Dark mode implementation details
The pre-commit (lines 177–187) and performance (lines 168–175) checklists with checkboxes enable developers to self-verify work before submission. Security guidelines (lines 189–195) cover input validation, authorization, parameterized queries (via Prisma), and file path sanitization—appropriate for a platform handling user submissions.
.claude/commands/review.md (1)
1-80: Comprehensive and well-structured review workflow.The review command defines clear evaluation criteria aligned with the PR objectives (Performance, No Regression, Maintainability) plus Security. The structured output format with markdown template and actionable verdict options will drive consistent quality gates.
.claude/commands/pre-commit.md (2)
24-27: Verifypnpm formatbehavior in pre-commit context.Pre-commit hooks should check/validate without modifying files. The
pnpm formatcommand typically auto-formats files in-place, which can cause unexpected changes to staged files. Confirm whether this should be a check-only operation (e.g.,pnpm format --check) that validates formatting without modification, or if auto-formatting-then-staging is the intended workflow.
44-77: Clear code quality gates and output structure.The scans for common issues (console.log, TODO without refs, hardcoded secrets, any-types) are practical and actionable. The output template with results table and specific file/line guidance provides clear visibility into quality status and remediation needs.
- Remove orphan closing code fence in test-coverage.md - Replace hardcoded path with git rev-parse --show-toplevel in Stop hook
Native buttons handle Enter/Space automatically. Updated example to show: - Simple onClick for native buttons (no redundant onKeyDown) - Proper keyboard handling for custom elements (role="button" with tabIndex, handling both Enter and Space keys)
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.claude/settings.json (1)
37-37: Consider adding error suppression to git rev-parse.The
cd "$(git rev-parse --show-toplevel)"command could produce errors if executed outside a git repository. While the subsequentgit diffhas2>/dev/null, thegit rev-parsedoes not.🔎 Proposed enhancement for error handling
{ "type": "command", - "command": "bash -c 'cd \"$(git rev-parse --show-toplevel)\" && if git diff --name-only HEAD 2>/dev/null | grep -qE \"\\.(ts|tsx)$\"; then echo \"[Reminder] TypeScript files changed. Consider running: pnpm check-types && pnpm lint && pnpm test\"; fi'" + "command": "bash -c 'cd \"$(git rev-parse --show-toplevel 2>/dev/null)\" 2>/dev/null && if git diff --name-only HEAD 2>/dev/null | grep -qE \"\\.(ts|tsx)$\"; then echo \"[Reminder] TypeScript files changed. Consider running: pnpm check-types && pnpm lint && pnpm test\"; fi'" }This suppresses errors when the hook runs in a non-git context, preventing noise in the output.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.claude/commands/test-coverage.md.claude/settings.json.claude/skills/component-development/SKILL.md
🚧 Files skipped from review as they are similar to previous changes (2)
- .claude/commands/test-coverage.md
- .claude/skills/component-development/SKILL.md
🔇 Additional comments (2)
.claude/settings.json (2)
31-41: Good fix for the hardcoded path issue!The Stop hook now correctly uses
git rev-parse --show-toplevelto dynamically resolve the repository root, addressing the past review concern. This approach ensures the hook works across different environments.
20-28: Verify the environment variable name for Claude Code hook integration.The PostToolUse hook references
$CLAUDE_TOOL_ARG_file_path, but this environment variable is not documented or referenced anywhere in the repository. Check Claude Code's official documentation to confirm the correct environment variable names for accessing tool arguments in hook commands. If the variable name is incorrect, the TypeScript file detection hook will fail silently.
Add two specialized subagents for automated code quality checks: 1. code-reviewer: Reviews code for performance, maintainability, and adherence to codebase patterns (Server Components, Prisma queries, API validation, TypeScript best practices) 2. security-reviewer: Identifies vulnerabilities including auth issues, input validation gaps, injection risks, data exposure, and OWASP Top 10 concerns specific to this platform Both agents are configured to be used proactively after code changes.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
.claude/agents/code-reviewer.md (1)
105-108: Clarify the "always use select" guidance for Prisma queries.The guideline states "Always use
selectto limit fields," but this lacks nuance for cases where all fields are needed or when explicit selection would reduce maintainability. Consider adding guidance on:
- When to use
selectvs. fetching all fields (e.g., performance thresholds, large vs. small tables)- How to handle schema changes (e.g., if a table gains many fields later)
- Trade-off between explicit selection (safety, performance) and implicit inclusion (maintainability)
This would help agents and developers apply the rule intelligently rather than mechanically.
.claude/agents/security-reviewer.md (3)
40-53: Scope overlap with code-reviewer: Input validation is covered by both agents.Both code-reviewer.md (line 33, "API Validation: Zod schemas for all API inputs?") and security-reviewer.md (lines 40-53) cover Zod input validation. While some redundancy is acceptable for agent design, consider clarifying whether:
- Code-reviewer should focus on completeness of validation (all inputs covered)
- Security-reviewer should focus on quality of validation (safe parsing, error handling, injection prevention)
This would reduce duplication and ensure agents complement each other rather than duplicate effort.
100-111: OWASP Top 10 coverage is good, but some critical topics are missing.The OWASP checklist is comprehensive, but several security concerns relevant to this codebase are not explicitly covered:
- HTTP Security Headers (CORS, CSP, X-Frame-Options, etc.)
- Rate Limiting & Brute-Force Protection (especially relevant for auth endpoints)
- Token Security (JWT expiration, refresh token rotation, secure storage)
- Secure Password Reset (e.g., time-limited tokens, verification emails)
- Account Enumeration (e.g., "user not found" vs. "invalid password" discrepancies)
Consider adding a section covering these attack vectors, or confirming that these are handled by a separate security layer (e.g., API gateway, reverse proxy).
70-77: Data Exposure example should include additional sensitive fields.The code example shows excluding
passwordHash,sessions, andaccounts, but a competitive programming platform may also collect sensitive data like:
- Email addresses (for password reset, account recovery)
- IP addresses / submission metadata (could identify users)
- Contest/submission status (may reveal personal performance or registration)
- Payment information (if applicable)
Expand the guidance to remind reviewers to audit all selected fields for inadvertent data exposure.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.claude/agents/code-reviewer.md.claude/agents/security-reviewer.md
🔇 Additional comments (3)
.claude/agents/code-reviewer.md (1)
90-92: All three pnpm commands referenced in the verification checklist (check-types,lint,test) are defined inpackage.jsonand available for use. The checklist is complete and accurate.Likely an incorrect or invalid review comment.
.claude/agents/security-reviewer.md (2)
79-84: No action needed—S3 is the actual file storage backend.The codebase exclusively uses AWS S3 with presigned URLs for file uploads (via
@aws-sdk/s3-request-presignerinsrc/app/api/tasks/route.tsand related files). No alternative storage backends are present. The File Handling section guidance is accurate and appropriately scoped to the actual implementation.Likely an incorrect or invalid review comment.
164-183: Security patterns in documentation are accurate—all verified against implementation.All three codebase-specific security patterns mentioned have been verified:
API Route Security:
getServerUser()from@/lib/sessionis confirmed as the canonical auth check across multiple API routes and server components (src/app/api/users/route.ts,src/app/api/user/route.ts, etc.).File Upload Security: Presigned S3 URLs via
getSignedUrlfrom@aws-sdk/s3-request-presignerare confirmed insrc/app/api/tasks/route.tsandsrc/app/api/tasks/[id]/route.ts.Submission Handling: Brotli compression is confirmed in
src/lib/codeTransformer.tsusingbrotliCompress/brotliDecompress, and submissions are decompressed insrc/app/api/submissions/[id]/route.ts.The documentation accurately reflects current implementation. No updates needed.
…regression
Add comprehensive Claude Code configuration focusing on three core principles:
Configuration includes:
Summary by CodeRabbit
Documentation
Chores
✏️ Tip: You can customize this high-level summary in your review settings.