Skip to content

Conversation

@NathanFlurry
Copy link
Member

No description provided.

@railway-app
Copy link

railway-app bot commented Jan 13, 2026

🚅 Deployed to the rivet-pr-3880 environment in rivet-frontend

Service Status Web Updated (UTC)
website ✅ Success (View Logs) Web Jan 14, 2026 at 7:29 am
frontend-inspector 😴 Sleeping (View Logs) Web Jan 14, 2026 at 6:47 am
frontend-cloud 😴 Sleeping (View Logs) Web Jan 14, 2026 at 6:44 am

@railway-app railway-app bot temporarily deployed to rivet-frontend / rivet-pr-3880 January 13, 2026 21:55 Destroyed
Copy link
Member Author


How to use the Graphite Merge Queue

Add the label merge-queue to this PR to add it to the merge queue.

You must have a Graphite account in order to use the merge queue. Sign up using this link.

An organization admin has enabled the Graphite Merge Queue in this repository.

Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.

This stack of pull requests is managed by Graphite. Learn more about stacking.

@pkg-pr-new
Copy link

pkg-pr-new bot commented Jan 13, 2026

More templates

@rivetkit/cloudflare-workers

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/cloudflare-workers@3880

@rivetkit/db

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/db@3880

@rivetkit/framework-base

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/framework-base@3880

@rivetkit/next-js

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/next-js@3880

@rivetkit/react

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/react@3880

rivetkit

pnpm add https://pkg.pr.new/rivet-dev/rivet/rivetkit@3880

@rivetkit/sql-loader

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/sql-loader@3880

@rivetkit/virtual-websocket

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/virtual-websocket@3880

@rivetkit/engine-runner

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/engine-runner@3880

@rivetkit/engine-runner-protocol

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/engine-runner-protocol@3880

commit: 3e3b172

@claude
Copy link

claude bot commented Jan 13, 2026

PR Review: Next.js to Astro Migration Spec

Overview

This is a comprehensive and well-structured migration specification document for moving the Rivet website from Next.js to Astro. The document provides clear guidance, examples, and implementation details.

Strengths

Documentation Quality

  • Excellent organization with clear table of contents and logical flow from setup through deployment
  • Thorough examples with multiple real-world migration examples (docs, blog, templates) showing before/after code
  • Practical guidance including copy commands, package.json scripts, and deployment checklists
  • Clear comparisons table showing Next.js vs Astro patterns

Technical Accuracy

  • Proper use of Astro content collections with glob loaders and Zod schemas
  • Correctly identifies the transition from JS exports to YAML frontmatter
  • Accurate mapping of generateStaticParams to getStaticPaths
  • Good analysis of which MDX plugins can be reused vs replaced

Completeness

  • Covers all major aspects: structure, content, routing, deployment, and testing
  • Includes edge cases like image handling, sitemap generation, and llms.txt
  • Provides multiple approaches where appropriate

Suggestions for Improvement

  1. Content Collection Schema (Line 132-138): The posts collection mentions image handling but doesn't implement it. Images are better handled via glob imports at build time. Consider adding a description field for consistency.

  2. TypeScript Imports (Line 100): reference is imported but never used. Remove unused imports.

  3. Error Handling: Examples don't show how to handle missing entries (404 pages). Consider adding error handling in route examples.

  4. Code Duplication (Lines 965-974): TOC generation logic is duplicated across examples. Extract into a utility function.

  5. Railway Config (Lines 796-802): The railway.json example may be unnecessary since Railpack auto-detects. Add note that this is only for custom builds.

  6. Plugin Migration Clarity (Lines 250-266): Add explicit action items for which plugins to keep, replace, or remove.

  7. Missing Topics: No mention of React Context migration or client-side hydration strategies (client:load, client:visible, etc).

  8. Testing Strategy: Phase 6 doesn't include specific test commands. Add pnpm build, pnpm astro check, etc.

  9. Performance: No discussion of performance implications, build times, or bundle size comparisons.

  10. Additional Content: Consider adding estimated effort/time for each phase, rollback strategy, and Common Pitfalls section.

Security Considerations

  • No security issues identified
  • Migration maintains static site generation (inherently secure)
  • Railway deployment with automatic HTTPS is good practice

Overall Assessment

Rating: 4.5/5

This is an excellent migration specification with clear, actionable guidance and comprehensive coverage. The document is production-ready with the minor refinements suggested above. It demonstrates strong understanding of both Next.js and Astro patterns.

Recommendation

APPROVE with minor suggestions for enhancement. This spec provides a solid foundation for executing the website migration. The suggestions above are enhancements, not blockers.

Great work!

@NathanFlurry NathanFlurry force-pushed the 01-13-chore_website_migrate_next_-_astro branch from 22b4f92 to a7a37b9 Compare January 14, 2026 04:00
@railway-app railway-app bot temporarily deployed to rivet-frontend / rivet-pr-3880 January 14, 2026 04:00 Destroyed
@claude
Copy link

claude bot commented Jan 14, 2026

PR Review: Website Migration from Next.js to Astro

Overview

This PR migrates the website from Next.js to Astro, a major architectural change involving 484 files with 8,859 additions and 20,134 deletions. The migration follows a detailed specification document and converts the site from a dynamic Next.js application to a statically-generated Astro site.

✅ Positive Observations

Architecture & Implementation

  1. Well-Planned Migration: The migrate-spec.md document shows excellent planning with 1:1 route parity requirements clearly documented
  2. Static-First Approach: Correctly configured as output: static in astro.config.mjs, maintaining the static deployment model
  3. Smart Content Organization: Using Astro content collections API for structured content management
  4. Build Integration: Custom generate-routes integration elegantly handles MDX processing and route generation at build time
  5. Component Reuse: Successfully migrated React components to work within Astro with appropriate client directives

Configuration & DevOps

  1. Docker Setup: Multi-stage Dockerfile properly structured with Node.js builder and Caddy server
  2. Caddyfile: Well-configured with appropriate caching, CORS for changelog.json, and security headers
  3. TypeScript Config: Properly extends Astro strict config with appropriate compiler options

⚠️ Critical Issues

1. Missing Generated Routes File

Only apiPages.json exists in website/src/generated/, but components import routes.json at website/src/components/DocsNavigation.tsx:5

Impact: Build may fail if routes.json is not generated before TypeScript compilation
Fix: Ensure the generate-routes integration runs before TypeScript checks, or add routes.json to .gitignore with a placeholder

2. TypeScript Strict Mode Disabled

Location: website/tsconfig.json:9 sets strict: false

This is concerning for a large migration. While pragmatic for getting things working, it reduces type safety. Consider a follow-up PR to enable strict mode gradually.

3. Unpublished Package Dependency

Location: website/package.json:28 uses pkg.pr.new URL

Using pkg.pr.new URLs in production is risky - not permanent, could break if PR is closed, not reproducible builds.
Action Required: Publish to npm or use a git URL

🔒 Security Concerns

4. Unsafe HTML Rendering

Multiple locations use dangerouslySetInnerHTML for syntax-highlighted code. Current implementation appears to be build-time only which is safe, but worth documenting.

5. Missing Security Headers

Caddyfile includes basic headers but missing: Content-Security-Policy, Referrer-Policy, Permissions-Policy
Recommendation: Add CSP header for defense-in-depth

📝 Code Quality Issues

6. Inconsistent Error Handling

Location: website/src/integrations/generate-routes.ts:143-148

Silent failures with empty titles could cause broken navigation. Consider more specific error messages, failing the build on critical pages, or tracking which pages failed.

7. Regex Complexity in MDX Cleaning

Location: website/src/integrations/generate-routes.ts:21-42

The cleanMdxContent function uses multiple regex patterns that may not handle all edge cases. Consider using a proper MDX parser or AST transformation.

8. Empty Catch Block Pattern

Location: website/src/mdx/rehype.ts:27-28

Empty catch blocks hide errors. Should at least log in development.

⚡ Performance Considerations

9. Build Performance Issue

Location: website/src/integrations/generate-routes.ts:139-149

Sequential processing of all MDX files could be slow. Consider parallel processing with Promise.all.

10. Duplicate Shiki Language

Location: website/src/mdx/rehype.ts:66-67 has json listed twice

🧪 Testing & Documentation

11. No Tests

No test files found for the website directory. For a migration of this scale, consider integration tests for route generation, snapshot tests for critical pages, and visual regression tests.

12. Missing Documentation

  • No README.md in website/ directory (deleted)
  • No migration notes for future developers
  • Build process not documented

📊 Statistics & Impact

  • Files Changed: 484
  • Lines Added: 8,859
  • Lines Deleted: 20,134
  • Net Change: -11,275 lines (23% reduction!)
  • New Pages: 30+ Astro pages
  • Components: ~158 TypeScript/TSX files maintained

🔍 Recommendations

High Priority (Must Fix)

  1. ✅ Fix missing routes.json generation issue
  2. ✅ Replace pkg.pr.new dependency with stable version
  3. ✅ Add error handling for critical page generation failures
  4. ⚠️ Test the build process end-to-end

Medium Priority (Should Fix)

  1. Add Content-Security-Policy headers
  2. Enable TypeScript strict mode (incremental)
  3. Add basic integration tests for route generation
  4. Parallelize MDX processing for faster builds
  5. Document the build process and architecture

Low Priority (Nice to Have)

  1. Remove duplicate json language from Shiki config
  2. Improve error messages in MDX cleaning
  3. Add development documentation (README)
  4. Consider AST-based MDX processing instead of regex

✨ Conclusion

This is a well-executed migration that successfully converts a Next.js site to Astro with significant code reduction. The architecture is sound, and the static-first approach is correct.

Blocking Issues (must fix):

  • Missing routes.json generation/gitignore
  • Unpublished package dependency

Strongly Recommended (should fix):

  • Better error handling in route generation
  • End-to-end build testing

The 23% code reduction while maintaining functionality is excellent! The migration spec document demonstrates thorough planning. With the critical issues addressed, this PR will be ready to merge.


Overall Assessment: ⚠️ Approve with Changes Required

Fix the blocking issues, test the build thoroughly, and this will be a great improvement to the codebase.

@NathanFlurry NathanFlurry force-pushed the 01-13-chore_website_migrate_next_-_astro branch from a7a37b9 to 5ba3ae8 Compare January 14, 2026 04:20
@railway-app railway-app bot temporarily deployed to rivet-frontend / rivet-pr-3880 January 14, 2026 04:20 Destroyed
@NathanFlurry NathanFlurry force-pushed the 01-13-chore_website_migrate_next_-_astro branch from 5ba3ae8 to 4910468 Compare January 14, 2026 06:20
@railway-app railway-app bot temporarily deployed to rivet-frontend / rivet-pr-3880 January 14, 2026 06:20 Destroyed
@NathanFlurry NathanFlurry force-pushed the 01-13-chore_website_migrate_next_-_astro branch from 4910468 to be733f7 Compare January 14, 2026 06:34
@railway-app railway-app bot temporarily deployed to rivet-frontend / rivet-pr-3880 January 14, 2026 06:34 Destroyed
@NathanFlurry NathanFlurry force-pushed the 01-13-chore_website_migrate_next_-_astro branch from be733f7 to aa43128 Compare January 14, 2026 06:36
@railway-app railway-app bot temporarily deployed to rivet-frontend / rivet-pr-3880 January 14, 2026 06:36 Destroyed
@NathanFlurry NathanFlurry force-pushed the 01-13-chore_website_migrate_next_-_astro branch from aa43128 to 612197c Compare January 14, 2026 06:40
@railway-app railway-app bot temporarily deployed to rivet-frontend / rivet-pr-3880 January 14, 2026 06:40 Destroyed
@claude
Copy link

claude bot commented Jan 14, 2026

PR Review: Next.js to Astro Migration

Overview

This is a massive migration PR moving the website from Next.js to Astro, affecting 489 files with +9,199/-20,284 lines of code. The migration includes:

  • Complete framework replacement (Next.js → Astro)
  • Static site generation with Caddy serving
  • 237 renamed files, 134 removed files, 55 new files
  • Major dependency updates and cleanup

✅ Strengths

Architecture & Planning

  • Excellent migration specification: The migrate-spec.md file provides comprehensive documentation with clear requirements for static-only rendering and 1:1 route parity
  • Proper static site approach: Correctly configured for static output with no SSR, matching the original deployment model
  • Well-structured content collections: Clean Astro content collections setup with proper schema validation

Code Quality

  • Clean component structure: React components properly migrated with client:load and client:visible directives for optimal hydration
  • Good separation of concerns: Clear distinction between Astro layouts, pages, and React components
  • Proper TypeScript usage: Maintained type safety throughout the migration

Infrastructure

  • Solid Docker setup: Multi-stage build properly configured with pnpm workspace support and LFS handling
  • Security headers: Caddyfile includes appropriate security headers (X-Frame-Options, X-Content-Type-Options, X-XSS-Protection)
  • Smart caching strategy: Aggressive caching for static assets, no-cache for HTML

Dependencies

  • Workspace integration: Correctly uses workspace dependencies for internal packages (@rivet-gg/components, @rivet-gg/icons, etc.)
  • Modern tooling: Uses latest Astro 5.x with appropriate integrations

⚠️ Issues & Concerns

Critical Issues

  1. No Test Coverage (High Priority)

    • Zero test files added or maintained in this migration
    • For a 489-file change touching critical infrastructure, this is risky
    • Recommendation: Add at minimum smoke tests for:
      • Critical page rendering
      • Route resolution
      • Content collection loading
      • Build process validation
  2. Unverified Dependency (Security)

    • @rivet-gg/cloud uses a pkg.pr.new URL: https://pkg.pr.new/rivet-dev/cloud/@rivet-gg/cloud@bf2ebb2
    • This points to a temporary preview package, not a stable version
    • Recommendation: Replace with a proper versioned release before merging
  3. CORS Configuration (Security)

    • Caddyfile CORS allows all *.rivet.dev subdomains without specific validation
    • While scoped to changelog.json, this could be tightened
    • Recommendation: Document why this broad access is needed or restrict further

Code Quality Issues

  1. Regex-Based MDX Cleaning (website/src/integrations/generate-routes.ts:21-42)

    • The cleanMdxContent() function uses fragile regex patterns to strip MDX/JSX syntax
    • Regex for JSX: /<[A-Z][a-zA-Z0-9]*[^>]*\/>/g won't handle edge cases (nested components, props with >)
    • Recommendation: Use a proper MDX/AST parser instead of regex for robust content extraction
  2. Magic Numbers (website/Dockerfile:36)

    • Cache mount ID s/3e31e381-166e-4a85-983a-a49830a88b96-/pnpm/store appears to be auto-generated
    • Recommendation: Use a semantic identifier like rivet-website-pnpm-store
  3. Error Handling Gaps

    • No error boundaries visible in key React components
    • The generate-routes.ts integration lacks error handling for file operations
    • Recommendation: Add error handling, especially in the build-time route generation

Documentation & Maintenance

  1. No Migration Guide

    • Missing documentation for:
      • How to run the site locally
      • How to add new pages/routes
      • Differences from Next.js workflow
    • Recommendation: Add a README in website/ with development instructions
  2. Dead Code Cleanup

    • Many files moved to public/examples-old/ instead of being deleted
    • migrate-spec.md should not be committed (it's a planning doc)
    • Recommendation: Remove old examples and spec file before merge

Performance Considerations

  1. Client-Side Hydration

    • Index page loads 9 components with client:load or client:visible
    • While appropriate, no performance budgets are documented
    • Recommendation: Audit bundle sizes and consider lazy loading strategies
  2. Build Time

    • 489 static pages with complex MDX processing could lead to long builds
    • No caching strategy visible for incremental builds
    • Recommendation: Monitor build times and consider build optimization if needed

Minor Issues

  1. Content Schema Flexibility

    • All content collection fields are .optional() in content.config.ts
    • This reduces type safety and makes required fields unclear
    • Recommendation: Mark truly required fields (like title) as required
  2. Environment Variables

    • Dockerfile has empty defaults for PUBLIC_POSTHOG_KEY, PUBLIC_TYPESENSE_HOST, etc.
    • Unclear if the site works without these
    • Recommendation: Document which env vars are required vs optional

🔍 Security Review

✅ Good

  • No secrets or credentials committed
  • Security headers properly configured
  • Static site reduces attack surface significantly

⚠️ Concerns

  • Preview dependency URL needs resolution
  • CORS policy is broad (documented above)
  • No Content-Security-Policy header configured
    • Recommendation: Consider adding CSP headers for defense-in-depth

📊 Testing Recommendations

Given the scale of this change, recommend:

  1. Pre-merge validation:

    • Verify all Next.js routes redirect/map correctly
    • Test all dynamic routes ([...slug], [slug]) render properly
    • Validate RSS/JSON feeds generate correctly
    • Check that examples load and work
    • Confirm search functionality (if applicable)
  2. Post-merge monitoring:

    • Monitor 404 rates for broken links
    • Check build time metrics
    • Validate SEO metadata preservation
    • Verify analytics still work (PostHog)

🎯 Recommendations Before Merge

Must Fix:

  1. Replace @rivet-gg/cloud preview dependency with stable version
  2. Add basic integration tests for route resolution
  3. Remove migrate-spec.md and public/examples-old/ directories

Should Fix:
4. Replace regex-based MDX cleaning with proper parser
5. Add website/README.md with development instructions
6. Make required content schema fields non-optional
7. Add error handling in build-time integrations

Nice to Have:
8. Add CSP headers to Caddyfile
9. Document performance budgets
10. Add comprehensive test suite

🏁 Overall Assessment

This is a well-executed migration with clear planning and solid architecture. The use of Astro for static generation is appropriate, and the code quality is generally good. However, the lack of testing for such a large change is concerning, and the preview dependency must be resolved.

Recommendation: Request changes to address critical issues (#1, #2) before merging. The other issues can be addressed in follow-up PRs if needed.


Review generated by Claude Code - Last updated: 2026-01-14T07:30:53Z

@NathanFlurry NathanFlurry force-pushed the 01-13-chore_website_migrate_next_-_astro branch from 612197c to 9f1f991 Compare January 14, 2026 06:44
@railway-app railway-app bot temporarily deployed to rivet-frontend / rivet-pr-3880 January 14, 2026 06:44 Destroyed
@NathanFlurry NathanFlurry force-pushed the 01-13-chore_website_migrate_next_-_astro branch from 9f1f991 to a5a5145 Compare January 14, 2026 07:17
@railway-app railway-app bot temporarily deployed to rivet-frontend / rivet-pr-3880 January 14, 2026 07:17 Destroyed
@NathanFlurry NathanFlurry force-pushed the 01-13-chore_website_migrate_next_-_astro branch from a5a5145 to 3e3b172 Compare January 14, 2026 07:27
@railway-app railway-app bot temporarily deployed to rivet-frontend / rivet-pr-3880 January 14, 2026 07:27 Destroyed
@NathanFlurry NathanFlurry marked this pull request as ready for review January 14, 2026 07:33
@graphite-app
Copy link
Contributor

graphite-app bot commented Jan 14, 2026

Merge activity

  • Jan 14, 7:34 AM UTC: NathanFlurry added this pull request to the Graphite merge queue.
  • Jan 14, 7:35 AM UTC: CI is running for this pull request on a draft pull request (#3892) due to your merge queue CI optimization settings.
  • Jan 14, 7:36 AM UTC: Merged by the Graphite merge queue via draft PR: #3892.

graphite-app bot pushed a commit that referenced this pull request Jan 14, 2026
@graphite-app graphite-app bot closed this Jan 14, 2026
@graphite-app graphite-app bot deleted the 01-13-chore_website_migrate_next_-_astro branch January 14, 2026 07:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants