Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughA new developer guidance file Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Pull request overview
This PR adds a CLAUDE.md file to provide guidance for Claude Code (claude.ai/code) when working with this repository. The file documents development commands, architecture patterns, and environment variables to help the AI assistant understand the codebase structure and conventions.
Key changes:
- Added comprehensive documentation covering development commands, monorepo structure, and key architectural patterns
- Documented the lazy initialization pattern used for external API clients to handle missing environment variables during CI builds
- Provided reference information for environment variables and their usage
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| / | ||
| ├── apps/ | ||
| │ └── web/ # Next.js 16 website (@onruntime/web) | ||
| ├── packages/ # Shared packages (future) |
There was a problem hiding this comment.
The documentation states that there is a packages/ directory for shared packages with the comment "(future)", but this directory does not currently exist in the repository. This could be misleading to someone using this documentation. Consider either removing this line entirely or making it clearer that this is a planned directory structure that doesn't exist yet (e.g., "packages/ (planned)" or removing it until the directory is actually created).
| ├── packages/ # Shared packages (future) | |
| ├── packages/ # Shared packages (planned, directory not yet created) |
| - **app/**: Next.js App Router pages and API routes | ||
| - **components/**: React components (ui/, layout/, marketing/) | ||
| - **services/**: External API clients with lazy initialization | ||
| - **constants/**: Static data (projects, agencies, services, team members) | ||
| - **content/**: MDX content (glossary, legal pages) | ||
| - **lib/**: Utilities and helpers | ||
| - **types/**: TypeScript type definitions |
There was a problem hiding this comment.
The Web App Structure section lists several directories but omits some that exist in the actual codebase (such as hooks/, logos/, screens/, and styles/). While not every directory needs to be documented, consider whether hooks/ and screens/ should be included for completeness, as they may contain important patterns for developers to understand.
| // services/email.ts - Proxy pattern for lazy init | ||
| export const resend = new Proxy({} as Resend, { | ||
| get(_, prop) { | ||
| if (!instance) { |
There was a problem hiding this comment.
The code example shows new Resend(env.RESEND_API_KEY) but the actual implementation in apps/web/src/services/email.ts includes additional error handling that checks if the API key exists before creating the Resend instance. The example would be more accurate if it included the error handling or added a comment noting that error handling is omitted for brevity.
| if (!instance) { | |
| if (!instance) { | |
| // Note: The real implementation checks that env.RESEND_API_KEY is defined | |
| // before creating the Resend instance; this example omits that error handling | |
| // for brevity. |
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
CLAUDE.md
🧰 Additional context used
🪛 markdownlint-cli2 (0.18.1)
CLAUDE.md
22-22: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Agent
🔇 Additional comments (1)
CLAUDE.md (1)
1-70: Developer guidance is comprehensive and well-structured.The file effectively documents monorepo architecture, setup commands, key patterns, and environment variables. The examples for lazy initialization and service patterns are practical and directly applicable.
| ``` | ||
| / | ||
| ├── apps/ | ||
| │ └── web/ # Next.js 16 website (@onruntime/web) | ||
| ├── packages/ # Shared packages (future) | ||
| ├── turbo.json # Turborepo configuration | ||
| └── pnpm-workspace.yaml # pnpm workspace config | ||
| ``` |
There was a problem hiding this comment.
Add language specifier to the directory tree code block.
The fenced code block showing the monorepo structure is missing a language identifier, which markdown linters require for best practices.
🔎 Proposed fix
-```
+```text
/
├── apps/
│ └── web/ # Next.js 16 website (@onruntime/web)
├── packages/ # Shared packages (future)
├── turbo.json # Turborepo configuration
└── pnpm-workspace.yaml # pnpm workspace config
-```
+```📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ``` | |
| / | |
| ├── apps/ | |
| │ └── web/ # Next.js 16 website (@onruntime/web) | |
| ├── packages/ # Shared packages (future) | |
| ├── turbo.json # Turborepo configuration | |
| └── pnpm-workspace.yaml # pnpm workspace config | |
| ``` |
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
22-22: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
In CLAUDE.md around lines 22 to 29 the fenced code block showing the directory
tree lacks a language specifier; update the opening fence to include a language
tag such as "text" (i.e. change ``` to ```text) so markdown linters recognize
it, leaving the block content unchanged and keeping the closing ``` as is.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.