refactor: performance improvements for inspector #3452

jog1t · 2025-11-12T19:57:20Z

No description provided.

vercel · 2025-11-12T19:57:25Z

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

Project	Deployment	Preview	Comments	Updated (UTC)
rivet-inspector	Error			Nov 13, 2025 7:03pm
rivetkit-serverless	Error			Nov 13, 2025 7:03pm

2 Skipped Deployments

Project	Deployment	Preview	Comments	Updated (UTC)
rivet-cloud	Ignored			Nov 13, 2025 7:03pm
rivet-site	Ignored	Preview		Nov 13, 2025 7:03pm

jog1t · 2025-11-12T19:57:50Z

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.

claude · 2025-11-12T19:59:18Z

Code Review - PR #3452: Performance Improvements for Inspector

Overview

This PR is a major refactor focusing on performance improvements for the inspector. The changes involve removing 129 files (9,810 deletions) and adding 988 lines, resulting in a significant simplification and modernization of the frontend codebase.

Summary

Strengths:

✅ Major cleanup: Removes a large amount of legacy code (129 files deleted)
✅ Modernization: Migrates from Jotai atom-based state to React Query data providers
✅ Better separation of concerns with dedicated data provider pattern
✅ Consistent query/mutation patterns with TanStack Query

Areas for Attention:

⚠️ Performance implications of query options need verification
⚠️ Missing type safety in some areas
⚠️ Potential API call inefficiencies
⚠️ Incomplete error handling

Detailed Findings

1. Code Quality & Architecture

✅ Good: Data Provider Pattern

The new data provider architecture in default-data-provider.tsx and engine-data-provider.tsx is well-structured:

Clear separation between default (stub) and engine implementations
Consistent query/mutation options factory pattern
Type-safe return types

⚠️ Concern: Query Key Type Safety

Location: frontend/src/app/data-providers/engine-data-provider.tsx:65

queryKey: ["namespaces"] as any,

Using as any defeats TypeScript's type checking. Should use proper QueryKey typing:

queryKey: ["namespaces"] as QueryKey,

2. Performance Considerations

⚠️ Potential Issue: Refetch Intervals

Location: frontend/src/app/data-providers/default-data-provider.tsx:57, 109

refetchInterval: 2000,  // Line 57
refetchInterval: 5000,  // Line 109

Concern: Different refetch intervals (2s vs 5s) for similar data could cause performance issues:

The actorsQueryOptions uses 2s refetch
The actorsListQueryOptions uses 5s refetch
Both query the same underlying data

Recommendation:

Document why different intervals are needed
Consider using a WebSocket or Server-Sent Events for real-time updates instead of aggressive polling
Make refetch intervals configurable or conditional based on user activity

⚠️ Potential Issue: Query Invalidation Predicate

Location: frontend/src/app/data-providers/default-data-provider.tsx:216-220

queryClient.invalidateQueries({
  predicate: (query) => {
    return keys.every((k) => query.queryKey.includes(k));
  },
});

Concern: This invalidation pattern may be overly broad and invalidate unrelated queries if key naming isn't carefully managed.

Recommendation: Use more specific query key matching or structured query keys.

3. Potential Bugs

🐛 Bug: Empty Query Always Returns Empty

Location: frontend/src/app/data-providers/engine-data-provider.tsx:230-244

if (
  (opts?.n?.length === 0 || \!opts?.n) &&
  (opts?.filters?.id?.value?.length === 0 ||
    \!opts?.filters?.id?.value ||
    opts?.filters.key?.value?.length === 0 ||
    \!opts?.filters.key?.value)
) {
  return {
    actors: [],
    pagination: { cursor: undefined },
  };
}

Issue: This logic seems incorrect. The condition returns empty results when:

No names AND (no ID filter OR no key filter)

This means if you provide an ID filter but no key filter, it still returns empty. This is likely a logic error. Should probably be:

if (
  (\!opts?.n || opts.n.length === 0) &&
  (\!opts?.filters?.id?.value || opts.filters.id.value.length === 0) &&
  (\!opts?.filters.key?.value || opts.filters.key.value.length === 0)
) {
  // Return empty only if ALL are empty

⚠️ Concern: Actor Transformation Missing Null Checks

Location: frontend/src/app/data-providers/engine-data-provider.tsx:636-660

The transformActor function assumes certain fields exist. Consider adding runtime validation or using optional chaining more defensively.

4. Missing Error Handling

⚠️ Concern: Generic Error Messages

Location: frontend/src/app/data-providers/default-data-provider.tsx:59, 74, 187, etc.

throw new Error("Not implemented");

While these are stub implementations, ensure that the actual implementations have proper error handling with descriptive messages.

⚠️ Concern: Silent Failures

Location: frontend/src/app/data-providers/engine-data-provider.tsx:92, 93, 151, etc.

retry: shouldRetryAllExpect403,
throwOnError: noThrow,

Using noThrow might hide errors from users. Ensure there's proper error state UI to display these to users.

5. Code Style & Best Practices

✅ Good: Consistent Patterns

Query options follow a consistent naming convention (*QueryOptions)
Mutation options follow the same pattern (*MutationOptions)
Good use of TypeScript for type safety (except where noted)

⚠️ Minor: Commented Code

Location: frontend/src/components/actors/actors-actor-details.tsx:150-170, 225-234

Several tabs are commented out (logs, metrics). Consider:

Removing if permanently disabled
Adding TODO comments explaining when they'll be re-enabled
Using feature flags if conditional

6. Testing Considerations

Concern: This is a massive refactor (129 files removed, significant architectural changes), but there's no mention of:

Test coverage for the new data providers
Integration tests for query/mutation flows
Migration testing to ensure no regressions

Recommendation:

Add unit tests for the data provider query/mutation options
Add integration tests for critical user flows
Manual testing checklist for regression testing

7. Performance Validation

Questions to verify:

Has the bundle size changed significantly?
What's the impact on initial load time?
How many API calls are made on initial page load now vs before?
Are there any query waterfalls introduced?

Recommendation: Run performance profiling before/after to validate improvements.

Security Concerns

✅ Good: Auth Handling

The mightRequireAuth metadata pattern is good for tracking which queries need authentication.

⚠️ Note: Header Manipulation

Location: frontend/src/app/data-providers/engine-data-provider.tsx:44-48

Object.keys(args.headers || {}).forEach((key) => {
  if (key.toLowerCase().startsWith("x-fern-")) {
    delete args.headers?.[key];
  }
});

Ensure this header removal is intentional and documented. If Fern headers are being stripped for security/privacy, add a comment explaining why.

Recommendations

High Priority

Fix the query filter logic at engine-data-provider.tsx:230-244
Replace as any with proper typing at engine-data-provider.tsx:65
Add error handling UI for queries with noThrow
Document refetch interval strategy and consider alternatives to aggressive polling

Medium Priority

Add unit tests for data provider functions
Clean up commented code or add TODO explanations
Document the header removal logic
Add query key constants to prevent invalidation bugs
Validate performance improvements with before/after metrics

Low Priority

Consider adding JSDoc comments for complex query options
Extract magic numbers (RECORDS_PER_PAGE) to shared constants
Add runtime type validation for API responses using Zod

Conclusion

This is a solid architectural improvement that modernizes the codebase and should improve maintainability. The removal of Jotai atoms in favor of React Query is a good move for this use case.

However, there are several bugs and performance concerns that should be addressed before merging, particularly:

The query filter logic bug
Type safety issues
Aggressive polling strategy

Overall Assessment: ✅ Approve with recommendations to address the high-priority issues.

Great work on this refactor! The codebase will be much cleaner after this lands. 🚀

pkg-pr-new · 2025-11-12T19:59:35Z

More templates

@rivetkit/actor

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

@rivetkit/cloudflare-workers

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

@rivetkit/core

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

@rivetkit/db

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

@rivetkit/framework-base

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

@rivetkit/next-js

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

@rivetkit/react

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

rivetkit

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

@rivetkit/sql-loader

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

@rivetkit/engine-runner

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

@rivetkit/engine-runner-protocol

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

commit: 18ed019

claude · 2025-11-13T19:04:54Z

PR Review: Performance Improvements for Inspector

This PR represents a substantial refactoring that removes 10,090 lines and adds 1,308 lines, achieving a net reduction of ~8,782 lines. The changes focus on cleaning up the actor/inspector component architecture.

✅ Positive Changes

Significant Code Reduction: The massive reduction in code complexity is excellent for maintainability. Removing obsolete components and consolidating logic reduces the attack surface and cognitive load.
Simplified Data Provider Architecture: The refactoring of data providers (default, engine, inspector) shows better separation of concerns with cleaner query options patterns using TanStack Query.
Commented Out Code Pattern: In actor-editable-state.tsx, the entire implementation is commented out rather than deleted. While this might be intentional during migration, it's cleaner to remove dead code entirely (the file can be restored from git history if needed).
Query Options Improvements: The new query structure using queryOptions and infiniteQueryOptions follows React Query best practices with proper stale time, retry logic, and error handling.

⚠️ Concerns & Recommendations

1. Missing Type Import (`inspector-data-provider.tsx`)

Lines 157-173 reference InspectorActor and Actor types that aren't imported. This will cause TypeScript errors:

// Missing imports at top of file:
import type { Actor, InspectorActor } from './types'; // or wherever these are defined

2. Missing Type Definitions (`engine-data-provider.tsx`)

Line 654 references CrashPolicy type without import:

crashPolicy: a.crashPolicy as CrashPolicy, // Line 654

Add the import:

import type { CrashPolicy } from '@/types'; // adjust path as needed

3. Error Handling in Default Provider

Lines 58-62 in default-data-provider.tsx use throw new Error("Not implemented") followed by unreachable return statements. While the biome-ignore comment acknowledges this, consider using a more type-safe approach:

queryFn: async (): Promise<Rivet.ActorsListResponse> => {
  throw new Error("Not implemented");
}

This makes the return type explicit and removes the unreachable code smell.

4. Authentication Context

Line 22 in engine-data-provider.tsx:

const mightRequireAuth = __APP_TYPE__ === "engine";

Ensure __APP_TYPE__ is properly defined globally (likely in vite config or types). This global reference should be documented or moved to a config file for clarity.

5. Test Coverage

With this much code deletion, verify that:

All existing tests still pass
Tests for removed components have been removed or updated
Critical data provider paths have test coverage

Run: npm test or pnpm test to verify.

6. Dead Code in `actor-editable-state.tsx`

The entire component implementation (lines 38-122) is commented out. If this feature is temporarily disabled, add a TODO comment explaining why and when it will be restored. Otherwise, delete the commented code:

// TODO(issue-XXXX): Re-enable state editing when backend supports PATCH endpoint
// or
// Remove this file entirely if no longer needed

🔍 Security Considerations

Token Handling: The createClient function in engine-data-provider.tsx (lines 35-52) properly handles authentication tokens. Good practice.
XSS Prevention: The data transformation functions properly escape/transform API data. No obvious injection vectors.
Error Exposure: Error messages don't expose sensitive information - good practice.

🚀 Performance Considerations

Query Deduplication: Using TanStack Query with proper queryKey arrays enables automatic deduplication and caching - excellent.
Refetch Intervals: Some queries use refetchInterval: 2000 (2 seconds) and refetchInterval: 5000. Consider making these configurable or increasing for production to reduce server load.
Pagination: Proper use of initialPageParam and getNextPageParam for infinite queries. The RECORDS_PER_PAGE = 10 constant is reasonable.

📝 Minor Code Style Issues

Following the CLAUDE.md conventions:

Import Organization: Imports at top of file ✅ (follows guideline)
Structured Logging: N/A for frontend code
Hard Tabs: Frontend uses standard formatting (not Rust)

Suggested Action Items

✅ Add missing type imports in inspector-data-provider.tsx
✅ Add missing CrashPolicy type import in engine-data-provider.tsx
⚠️ Clarify or remove commented code in actor-editable-state.tsx
✅ Run full test suite to ensure no regressions
⚠️ Document the __APP_TYPE__ global or move to config

Overall Assessment

LGTM with minor fixes required ✅

The refactoring significantly improves code quality and maintainability. The missing type imports should be fixed before merging, but otherwise this is excellent work. The performance improvements from removing obsolete components and simplifying the data layer are valuable.

Nice work on the substantial cleanup! 🎉

claude · 2025-11-13T19:05:50Z

Code Review: Performance Improvements for Inspector

Summary

This PR performs a massive cleanup of the frontend codebase, removing 10,090 lines and adding only 1,308 lines (net deletion of ~8,800 lines). The primary focus is on deleting legacy actor management components from frontend/packages/components/src/actors/ and streamlining data providers.

Key Changes

1. Large-Scale File Deletions (70+ files)

The PR removes an extensive set of actor-related UI components, including:

Actor detail views and tabs (config, connections, logs, metrics, state, console)
Actor list components and layouts
Actor creation/management forms
Actor worker/REPL functionality
Numerous helper components (status indicators, tags, filters, etc.)

Concern: This is a breaking change that removes significant functionality. The PR title suggests "performance improvements," but this appears to be a feature removal rather than optimization.

2. Data Provider Refactoring

default-data-provider.tsx (new file):

Creates a clean abstraction layer for data fetching with query options
Implements proper typing with Zod schemas
Uses React Query patterns effectively
Good: Well-structured with consistent patterns

engine-data-provider.tsx (modified):

Simplified from 696 lines to 686 lines
Implements the default provider interface
Adds namespace context management
Good: Better separation of concerns

3. Commented-Out Code

actor-editable-state.tsx:

export function ActorEditableState({ state, actorId }: ActorEditableStateProps) {
	// return
	// const [isEditing, setIsEditing] = useState(false);
	// ... [entire implementation commented out]
}

actor-clear-events-log-button.tsx:

export function ActorClearEventsLogButton({ actorId }: { actorId: ActorId }) {
	// const { mutate, isPending } = useActorClearEventsMutation(actorId);
	return null;
	// ... [entire implementation commented out]
}

Critical Issue: Multiple components are gutted with their implementations commented out. This is not production-ready code.

Issues & Concerns

🔴 Critical Issues

Commented-Out Code: Several files contain entirely commented-out implementations that just return null. This is a code smell and suggests incomplete work:
- actor-editable-state.tsx
- actor-clear-events-log-button.tsx
CI Failures: All TypeScript checks are failing:
- tsc: FAILURE
- quality: FAILURE
- publish: FAILURE
- Vercel deployments for rivet-inspector and rivetkit-serverless: FAILURE
Breaking Changes: The massive deletion of components will break:
- Any code importing these deleted components
- User workflows depending on these features
- Integration tests (if they exist)
Missing Description: The PR has no description explaining:
- What performance improvements are being made
- Why these files are being removed
- What functionality is being preserved/replaced
- Migration path for affected code

⚠️ Major Concerns

Unclear Performance Impact: Despite the title "performance improvements for inspector," it's unclear:
- What performance metrics improved
- What was causing performance issues
- How removing these components improves performance
- Whether this is bundle size optimization or runtime performance
No Test Coverage Verification: With such extensive deletions, there's no evidence of:
- Updated tests
- Removed tests for deleted functionality
- Integration test updates
Import Path Changes: Files like actor-database.tsx show changes in how types are imported, but without seeing the full diff context, it's hard to verify all references are updated.

💡 Minor Issues

Type Assertions: In engine-data-provider.tsx:636-659, the transformActor function performs several unsafe transformations without validation.
Query Key Typing: Using as QueryKey and as any type assertions suggests loose typing that could cause runtime issues:
```
queryKey: ["namespaces"] as any,  // Line 65
```
Error Handling: Several query functions use noThrow and shouldRetryAllExpect403 without clear documentation of error states.

Recommendations

Before Merging

Fix CI Failures: All TypeScript compilation errors must be resolved
Remove or Implement Commented Code: Either remove the gutted components or implement them properly
Add PR Description: Explain the "why" behind these changes
Verify Breaking Changes: Ensure no existing code depends on deleted components
Add Tests: Verify test coverage for remaining functionality

Code Quality

Remove Type Assertions: Replace as any and excessive as QueryKey with proper typing
Add Error Boundaries: The new data provider should handle errors gracefully
Document Performance Gains: Add metrics/benchmarks showing the performance improvements
Migration Guide: If this removes user-facing features, provide migration documentation

Architecture

Feature Flags: Consider using feature flags for such large-scale removals
Incremental Rollout: Breaking this into smaller PRs would be safer
Deprecation Path: Mark features as deprecated before removal

Security Considerations

No security issues identified. The changes are primarily structural and don't introduce obvious vulnerabilities.

Performance Considerations

Potential Positives:

Smaller bundle size (~8,800 lines removed)
Fewer React components to load
Simplified data fetching layer

Unknown:

Runtime performance impact
Whether removed features are replaced by more efficient alternatives
Actual metrics on performance improvements

Verdict

❌ NOT READY FOR MERGE

This PR has failing CI checks, commented-out code, and no description explaining the changes. While the code cleanup appears well-intentioned, it needs significant work before merging.

Must Fix Before Merge:

✅ Resolve all TypeScript compilation errors
✅ Remove or properly implement commented-out code
✅ Add comprehensive PR description
✅ Verify no breaking changes to existing features
✅ Update or remove related tests

Recommended Improvements:

Add performance benchmarks/metrics
Consider breaking into smaller, focused PRs
Add migration guide if removing user-facing features
Improve type safety (remove as any assertions)

Reviewed by: Claude Code
Note: This is an automated review. Please verify all findings with human review.

jog1t mentioned this pull request Nov 12, 2025

fix(fe): use publishable token for integrating next.js with other services #3451

Draft

vercel bot had a problem deploying to Preview – rivet-inspector November 12, 2025 19:58 Failure

vercel bot had a problem deploying to Preview – rivetkit-serverless November 12, 2025 19:58 Failure

refactor: performance improvements for inspector

64a1aba

jog1t changed the base branch from 11-12-fix_fe_use_publishable_token_for_integrating_next.js_with_other_services to graphite-base/3452 November 13, 2025 19:02

jog1t force-pushed the graphite-base/3452 branch from 8cccc54 to 9031170 Compare November 13, 2025 19:03

jog1t force-pushed the 11-12-refactor_performance_improvements_for_inspector branch from 18ed019 to 64a1aba Compare November 13, 2025 19:03

vercel bot had a problem deploying to Preview – rivet-inspector November 13, 2025 19:03 Failure

vercel bot had a problem deploying to Preview – rivetkit-serverless November 13, 2025 19:03 Failure

jog1t changed the base branch from graphite-base/3452 to 11-12-chore_rivetkit_fix_type_checks November 13, 2025 19:03

NathanFlurry changed the base branch from 11-12-chore_rivetkit_fix_type_checks to graphite-base/3452 November 13, 2025 20:53

This was referenced Nov 13, 2025

chore(rivetkit): implement new hibernating ws protocol #3464

Open

feat(cloudflare-workers): add createInlineClient #3466

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

refactor: performance improvements for inspector #3452

refactor: performance improvements for inspector #3452

Uh oh!

jog1t commented Nov 12, 2025

Uh oh!

vercel bot commented Nov 12, 2025 •

edited

Loading

Uh oh!

jog1t commented Nov 12, 2025 •

edited by NathanFlurry

Loading

Uh oh!

claude bot commented Nov 12, 2025

Uh oh!

pkg-pr-new bot commented Nov 12, 2025

Uh oh!

claude bot commented Nov 13, 2025

Uh oh!

claude bot commented Nov 13, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

refactor: performance improvements for inspector #3452

Are you sure you want to change the base?

refactor: performance improvements for inspector #3452

Uh oh!

Conversation

jog1t commented Nov 12, 2025

Uh oh!

vercel bot commented Nov 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jog1t commented Nov 12, 2025 • edited by NathanFlurry Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

How to use the Graphite Merge Queue

Uh oh!

claude bot commented Nov 12, 2025

Code Review - PR #3452: Performance Improvements for Inspector

Overview

Summary

Detailed Findings

1. Code Quality & Architecture

✅ Good: Data Provider Pattern

⚠️ Concern: Query Key Type Safety

2. Performance Considerations

⚠️ Potential Issue: Refetch Intervals

⚠️ Potential Issue: Query Invalidation Predicate

3. Potential Bugs

🐛 Bug: Empty Query Always Returns Empty

⚠️ Concern: Actor Transformation Missing Null Checks

4. Missing Error Handling

⚠️ Concern: Generic Error Messages

⚠️ Concern: Silent Failures

5. Code Style & Best Practices

✅ Good: Consistent Patterns

⚠️ Minor: Commented Code

6. Testing Considerations

7. Performance Validation

Security Concerns

✅ Good: Auth Handling

⚠️ Note: Header Manipulation

Recommendations

High Priority

Medium Priority

Low Priority

Conclusion

Uh oh!

pkg-pr-new bot commented Nov 12, 2025

Uh oh!

claude bot commented Nov 13, 2025

PR Review: Performance Improvements for Inspector

✅ Positive Changes

⚠️ Concerns & Recommendations

1. Missing Type Import (inspector-data-provider.tsx)

2. Missing Type Definitions (engine-data-provider.tsx)

3. Error Handling in Default Provider

4. Authentication Context

5. Test Coverage

6. Dead Code in actor-editable-state.tsx

🔍 Security Considerations

🚀 Performance Considerations

📝 Minor Code Style Issues

Suggested Action Items

Overall Assessment

Uh oh!

claude bot commented Nov 13, 2025

Code Review: Performance Improvements for Inspector

Summary

Key Changes

1. Large-Scale File Deletions (70+ files)

2. Data Provider Refactoring

3. Commented-Out Code

Issues & Concerns

🔴 Critical Issues

⚠️ Major Concerns

💡 Minor Issues

Recommendations

Before Merging

Code Quality

Architecture

Security Considerations

Performance Considerations

vercel bot commented Nov 12, 2025 •

edited

Loading

jog1t commented Nov 12, 2025 •

edited by NathanFlurry

Loading

1. Missing Type Import (`inspector-data-provider.tsx`)

2. Missing Type Definitions (`engine-data-provider.tsx`)

6. Dead Code in `actor-editable-state.tsx`