Skip to content

Conversation

@NathanFlurry
Copy link
Member

No description provided.

@vercel
Copy link

vercel bot commented Nov 10, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Preview Comments Updated (UTC)
rivet-site Error Error Nov 13, 2025 9:40pm
rivetkit-serverless Error Error Nov 13, 2025 9:40pm
2 Skipped Deployments
Project Deployment Preview Comments Updated (UTC)
rivet-cloud Ignored Ignored Nov 13, 2025 9:40pm
rivet-inspector Ignored Ignored Preview Nov 13, 2025 9:40pm

This was referenced Nov 10, 2025
Copy link
Member Author

NathanFlurry commented Nov 10, 2025


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.

@NathanFlurry NathanFlurry force-pushed the 11-10-chore_rivetkit_remove_deprecated_packages branch from ecae19f to aaf96c9 Compare November 13, 2025 20:53
@NathanFlurry NathanFlurry force-pushed the 11-10-feat_site_add_typedoc_docs branch from 1969775 to 0e6327a Compare November 13, 2025 20:54
@vercel
Copy link

vercel bot commented Nov 13, 2025

Deployment failed with the following error:

Too many requests - try again in 1 minute (more than 120, code: "api-deployments-flood-pro").

Learn More: https://vercel.com/docs/limits#rate-limits

@claude
Copy link

claude bot commented Nov 13, 2025

Pull Request Review: TypeDoc Documentation Generation

Overview

This PR adds TypeDoc-based API documentation generation for RivetKit packages. The changes include a new script to generate TypeDoc docs, package configuration updates, and generated documentation output.


Positive Aspects

Well-Structured Script

The gen-typedoc.ts script (website/scripts/gen-typedoc.ts:1-329) demonstrates solid engineering:

  • Clear separation of concerns with focused helper functions
  • Good error handling with try-catch blocks
  • Helpful console logging for debugging
  • Automatic cleanup of temporary config files

Smart Entry Point Resolution

The logic to map .d.ts files back to source files (website/scripts/gen-typedoc.ts:47-67) is clever and handles multiple build output patterns well.

Proper Git Attribution

The .gitattributes updates correctly mark generated files as linguist-generated=true to exclude them from repository statistics.


Issues and Recommendations

Critical: Generated Files in Git

Issue: This PR commits 8,778 lines of generated TypeDoc HTML/CSS/JS files to the repository.

Problems:

  1. Repository bloat: Generated docs significantly increase repo size
  2. Merge conflicts: Any upstream changes to TypeDoc or source code will cause conflicts
  3. Review difficulty: Generated code obscures meaningful changes in PRs
  4. Maintenance burden: Every docs change requires committing thousands of lines

Recommendation:

  • Add website/public/typedoc/ to .gitignore
  • Generate docs during CI/CD build pipeline instead
  • If docs need to be committed (e.g., for GitHub Pages), use a separate branch or repository

Missing CI/CD Integration

Issue: The gen:typedoc script is defined in package.json but not integrated into any build process.

Recommendation: Add to prebuild script to ensure TypeDoc is run before deployments so docs stay current.

No Test Coverage

Issue: The complex gen-typedoc.ts script has no tests despite handling file system operations, package.json parsing, path mapping logic, and TypeScript configuration generation.

Recommendation: Add unit tests for extractDtsFiles(), mapDtsToSource(), and getEntryPoints() to cover the critical path mapping logic.

Error Handling Could Be More Robust

Issue: Some error cases are silently ignored in the cleanup function (website/scripts/gen-typedoc.ts:257-267).

Recommendation: Log errors even if they are non-fatal to aid debugging.

Minor: Hardcoded Memory Limit

Issue: NODE_OPTIONS="--max-old-space-size=8192" is hardcoded (website/scripts/gen-typedoc.ts:307).

Recommendation: Make this configurable via environment variable.


Security Considerations

No security issues identified. The script only reads from workspace packages, has no external network calls, no arbitrary code execution, and file operations are scoped to project directory.


Performance Considerations

Memory allocation is high (8GB) but likely necessary for TypeDoc. Consider running generation in separate build step/container and caching generated docs to avoid regeneration.


Code Quality Assessment

Overall: Good quality code with room for improvement

Strengths:

  • Clear, readable code with good naming
  • Proper async/await usage
  • Type safety with TypeScript interfaces
  • Comprehensive logging

Areas for Improvement:

  • Add JSDoc comments for exported functions
  • Extract magic strings to constants
  • Add input validation for public functions
  • Increase test coverage

Recommendations Summary

Must Fix (Before Merge)

  1. Remove generated files from git - Add typedoc output to .gitignore
  2. Integrate into build process - Run during CI/CD, not manually

Should Fix (Soon After)

  1. Add test coverage for critical path mapping logic
  2. Improve error handling and logging
  3. Document script usage and configuration options

Nice to Have

  1. Make memory limit configurable
  2. Add TypeScript strict null checks
  3. Extract constants and improve maintainability

Great work on implementing this feature! The core logic is solid. Addressing the generated files issue and CI integration will make this production-ready.

@graphite-app
Copy link
Contributor

graphite-app bot commented Nov 13, 2025

Merge activity

@claude
Copy link

claude bot commented Nov 13, 2025

PR Review: TypeDoc Documentation Generation

This PR adds TypeDoc-generated API documentation for RivetKit packages. Overall, this is a valuable addition for developer experience, but there are several considerations:

✅ Strengths

  1. Comprehensive Coverage: Generates documentation for all @rivetkit/* packages with exports
  2. Smart Source Detection: The script intelligently maps .d.ts files to source .ts files for better documentation (website/scripts/gen-typedoc.ts:47-67)
  3. Proper Git Attributes: Added linguist-generated=true for generated files (.gitattributes:22), correctly excluding them from language statistics
  4. LLM Integration: Added TypeDoc docs to llms.txt for AI-assisted development support
  5. Build Integration: Added gen:typedoc script to package.json for easy regeneration

⚠️ Concerns & Recommendations

1. Large Commit Size (1,047 files changed)

  • Issue: This PR commits 1,030+ generated HTML files directly to the repository
  • Recommendation: Consider these alternatives:
    • Generate docs during CI/CD and deploy to GitHub Pages or a CDN
    • Add website/public/typedoc/**/* to .gitignore and only commit the generation script
    • Use a separate gh-pages branch for documentation
  • Why: Generated files bloat the repository, slow down git operations, and create merge conflicts

2. Missing Documentation

  • Issue: No README or comments explaining how to regenerate the docs
  • Recommendation: Add a comment in package.json or a small README section:
    "gen:typedoc": "tsx scripts/gen-typedoc.ts", // Generates TypeDoc API docs to website/public/typedoc/

3. Build Process Integration

  • Question: Should this run automatically during pnpm build or pnpm gen?
  • Current State: The script exists but isn't part of the main build/gen workflow
  • Recommendation: Either:
    • Add to gen script if docs should always be fresh
    • Document when/how to run it manually
    • Run it only in CI for production builds

4. Error Handling in Script

  • Issue: The script at website/scripts/gen-typedoc.ts:96 hardcodes special handling for @rivetkit/engine-api-full due to "source compilation errors"
  • Recommendation:
    • Document why this package needs special treatment
    • Consider fixing the underlying source issues if possible
    • Add a comment explaining the workaround

5. Memory Configuration

  • Observation: The script uses --max-old-space-size=8192 (8GB) (website/scripts/gen-typedoc.ts:307)
  • Question: Is this necessary for all environments? Document if this is a known requirement

6. TypeScript Configuration

  • Good: Creates temporary typedoc.json and tsconfig.typedoc.json files and cleans them up
  • Good: Extends existing tsconfig when available

🔍 Code Quality

The generation script (website/scripts/gen-typedoc.ts) is well-written:

  • ✅ Proper async/await usage
  • ✅ Good error handling with try/catch
  • ✅ Cleanup logic in finally block
  • ✅ Clear logging for debugging
  • ✅ Type safety with interfaces

🧪 Testing

  • Missing: No indication that the generated docs were tested in the website
  • Recommendation: Verify:
    • Navigation works correctly
    • Search functionality works
    • Links between modules are correct
    • Mobile responsiveness

📊 Performance Impact

  • Repository Size: Adds ~9MB of generated HTML files
  • Build Time: TypeDoc generation with 8GB memory suggests this could be slow
  • CI Impact: If running in CI, ensure sufficient resources

🔐 Security

  • ✅ No sensitive information detected
  • ✅ Uses standard npm packages (typedoc)
  • ✅ No arbitrary code execution risks

💡 Overall Recommendation

Conditional Approval - This is good work, but consider addressing the repository bloat issue. I'd recommend:

  1. Option A (Recommended): Don't commit generated files, generate during deployment
  2. Option B: If you must commit them, ensure the .gitattributes settings properly exclude them from diffs/searches

The script quality is excellent and the integration is thoughtful. The main decision is about repository hygiene.


Generated by Claude Code

@claude
Copy link

claude bot commented Nov 13, 2025

Code Review - PR #3446: Add TypeDoc Documentation

This PR adds TypeDoc-generated API documentation for RivetKit packages. Here's my analysis:

Overview

This PR introduces a comprehensive TypeDoc generation system that:

  • Adds ~1,030 HTML documentation files for RivetKit packages
  • Creates a script to auto-generate docs from TypeScript source/declarations
  • Updates .gitattributes to mark generated files appropriately
  • Adds generated documentation files to the website's public directory

Positive Aspects

  1. Well-Structured Generation Script (website/scripts/gen-typedoc.ts)

    • Smart entry point detection from package.json exports
    • Prefers source .ts files over .d.ts for better documentation
    • Handles special cases (e.g., @rivetkit/engine-api-full uses .d.ts due to compilation issues)
    • Proper cleanup of temporary config files
    • Good error handling and logging
  2. Git Attribution

    • Correctly marks generated files as linguist-generated=true in .gitattributes
    • Helps keep repository statistics accurate
  3. Package Integration

    • Added typedoc as a dev dependency (v0.28.14)
    • Script added to package.json as gen:typedoc
    • Properly integrated with the existing build system

Concerns & Recommendations ⚠️

1. Repository Size Impact 🔴 Critical

  • Issue: Adding 8,778 lines and 20MB of generated HTML files to the repository
  • Impact:
    • Increases clone time and repository size
    • Creates noise in diffs and git history
    • Makes code reviews harder when docs are regenerated
  • Recommendation:
    • Generate TypeDoc during the build/deploy process instead of checking it in
    • Add website/public/typedoc/ to .gitignore
    • Update CI/CD pipeline to run pnpm gen:typedoc during deployment
    • This follows the pattern used by many projects (e.g., how you handle other generated content)

2. Documentation Maintenance 🟡 Medium

  • Issue: No indication of when/how docs should be regenerated
  • Recommendations:
    • If keeping docs in repo: Add a pre-commit hook or CI check to ensure docs are up-to-date
    • Document when developers should run pnpm gen:typedoc
    • Consider adding a check in CI that fails if docs are out of sync with source

3. Script Improvements 🟡 Medium

In website/scripts/gen-typedoc.ts:

// Line 96: Hard-coded package name for special handling
const preferDts = packageName === "@rivetkit/engine-api-full";
  • Issue: Magic string without explanation of why this package needs special treatment
  • Suggestion: Add a comment explaining the compilation issues or consider a more maintainable approach:
    // Packages that have source compilation errors and must use .d.ts files
    const DTS_ONLY_PACKAGES = new Set(["@rivetkit/engine-api-full"]);
    const preferDts = DTS_ONLY_PACKAGES.has(packageName);
// Line 307: Large memory allocation without explanation
NODE_OPTIONS="--max-old-space-size=8192"
  • Issue: 8GB memory allocation is quite high
  • Suggestion: Add a comment explaining why this is needed, or investigate if TypeDoc config can be optimized to reduce memory usage

4. Error Handling 🟢 Minor

// Lines 292-295: Silent error handling
try {
    await fs.rm(outputPath, { recursive: true, force: true });
} catch (error) {
    // Ignore errors if directory doesn't exist
}
  • Suggestion: While this is probably fine, consider logging unexpected errors (not ENOENT)

5. Generated File Quality 🟢 Minor

Looking at the generated HTML:

  • Files are well-structured with proper navigation
  • Source links point to GitHub (good!)
  • Includes proper TypeScript type information
  • However: Title is generic "Documentation" - consider customizing to "RivetKit API Documentation"

Security Considerations

  • No security issues identified
  • Generated files are static HTML
  • No sensitive information exposed

Performance Considerations 🟡

  • 1,030 HTML files + assets = 20MB could impact website load times if not properly optimized
  • Ensure website hosting serves these with appropriate caching headers
  • Consider lazy-loading or splitting documentation by package

Testing Recommendations 🔵

  1. Verify all generated links work correctly
  2. Test documentation search functionality
  3. Ensure mobile responsiveness of generated docs
  4. Test that source links to GitHub work properly
  5. Validate that all RivetKit packages are included

Action Items Summary

Must Address:

  1. Decision needed: Should generated docs be checked into git or generated during build?
    • If build-time: Add to .gitignore, update CI/CD
    • If checked in: Add pre-commit hook or CI validation

Should Address:
2. Add comments explaining special cases (preferDts, memory allocation)
3. Document the regeneration workflow for developers
4. Customize page title from "Documentation" to "RivetKit API Documentation"

Nice to Have:
5. Add more granular error handling in cleanup code
6. Consider breaking docs by package for better performance
7. Add tests to validate doc generation

Recommendation

This is a valuable addition to the project, but I recommend not merging as-is. The biggest concern is checking in generated files. I suggest:

  1. Move doc generation to build-time (preferred approach)
  2. OR add automation to keep docs in sync if they must be checked in

The generation script itself is well-written and thoughtful. With the repository size concern addressed, this would be a great addition!


Generated with Claude Code 🤖

Base automatically changed from 11-10-chore_rivetkit_remove_deprecated_packages to main November 13, 2025 22:25
@NathanFlurry NathanFlurry merged commit 089474e into main Nov 13, 2025
10 of 18 checks passed
@NathanFlurry NathanFlurry deleted the 11-10-feat_site_add_typedoc_docs branch November 13, 2025 22:25
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.

3 participants