Skip to content

Conversation

@graphite-app
Copy link
Contributor

@graphite-app graphite-app bot commented Nov 22, 2025

This draft PR was created by the Graphite merge queue.
Trunk will be fast forwarded to the HEAD of this PR when CI passes, and the original PRs will be closed.

The following PRs are included in this draft PR:

@vercel
Copy link

vercel bot commented Nov 22, 2025

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

Project Deployment Preview Comments Updated (UTC)
rivet-site Ready Ready Preview Comment Nov 22, 2025 1:17am
rivetkit-serverless Ready Ready Preview Comment Nov 22, 2025 1:17am
2 Skipped Deployments
Project Deployment Preview Comments Updated (UTC)
rivet-cloud Ignored Ignored Nov 22, 2025 1:17am
rivet-inspector Ignored Ignored Preview Nov 22, 2025 1:17am

@claude
Copy link

claude bot commented Nov 22, 2025

PR Review: Merge Queue PRs 3525, 3526

This PR combines two changes:

Summary

This is a significant cleanup PR that removes outdated/deprecated examples and reorganizes the documentation structure. The PR removes ~7,600 lines and adds ~5,500 lines, with most additions being reorganized quickstart examples.


✅ What Looks Good

1. Examples Cleanup (PR #3525)

  • Removes several deprecated examples that appear to be outdated or redundant:
    • background-jobs, bots, better-auth-external-db
    • cloudflare-workers-inline-client, counter-next-js
    • crdt, database, game
  • Keeps examples clean and focused for users

2. New Quickstart Examples

  • The new quickstart examples (quickstart-actions, quickstart-cross-actor-actions, quickstart-state, quickstart-realtime, quickstart-scheduling, quickstart-native-websockets, quickstart-multi-region) are well-structured and demonstrate RivetKit features effectively
  • Good test coverage with comprehensive test suites for each example
  • Clean separation of concerns between actors

3. Code Quality in New Examples

  • TypeScript interfaces are properly defined
  • Actions are clearly documented with comments
  • State management follows RivetKit patterns correctly
  • Cross-actor communication patterns are demonstrated well

⚠️ Suggestions

1. quickstart-actions/src/backend/registry.ts:117-124
The createEmployee method returns a manually constructed EmployeeProfile with a new hiredAt timestamp instead of fetching the actual profile from the created actor:

return {
  employeeId,
  name: employeeData.name,
  email: employeeData.email,
  position: employeeData.position,
  companyId: c.state.profile.id,
  hiredAt: Date.now(),  // This timestamp may differ from the one in the actor
};

This could lead to inconsistencies if the actor's createState runs with a slightly different timestamp. Consider fetching the profile from the newly created actor instead.

2. quickstart-scheduling/src/backend/registry.ts:100-111
The getStats method has a logic issue:

const total = c.state.reminders.length;
const completed = c.state.completedCount;
const pending = total - completed;  // This can be negative or incorrect

completedCount tracks all-time completions, but total is the current reminders length. Once reminders are removed, this math doesn't work correctly. Consider tracking completedReminders separately or calculating pending differently.

3. kitchen-sink/src/backend/actors/websocket.ts - Type Safety

The file uses any types extensively:

  • c: any on lines 3, 179, 186, 191
  • opts: any on line 6
  • event: any on lines 39, 173

Consider using proper RivetKit types for better type safety.

4. quickstart-native-websockets/src/backend/registry.ts:107-114
The WebSocket cleanup on close doesn't remove the cursor from persistent state:

websocket.addEventListener("close", () => {
  console.log(`websocket disconnected: userId=${userId}`);
  c.vars.websockets.delete(userId);
  // Missing: delete c.state.cursors[userId];
});

Stale cursor data will persist in state after disconnection.


🔒 Security Considerations

  • No security concerns identified
  • Examples don't expose sensitive data patterns
  • WebSocket validation properly checks for missing userId parameter

📊 Test Coverage

  • New quickstart examples include comprehensive test suites
  • Tests cover happy paths and edge cases (e.g., insufficient inventory, multiple concurrent checkouts)
  • Good use of setupTest from rivetkit/test

✅ Overall Assessment

This is a clean housekeeping PR that improves the examples directory by removing outdated code and adding well-structured quickstart examples. The suggestions above are minor improvements that could be addressed in follow-up PRs.

Recommendation: Approve with the understanding that the minor issues noted above can be addressed separately if needed.

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