Skip to content

Conversation

@jog1t
Copy link
Contributor

@jog1t jog1t commented Jan 19, 2026

Closes FRONT-915

TL;DR

Improved UI components and error handling for the Rivet application, with focus on sidebar layout, actor error handling, and query caching.

What changed?

  • Sidebar Layout: Restructured the sidebar navigation with proper sizing and layout for buttons, wrapped links in a flex container, and added proper icon placement
  • Actor Error Handling: Enhanced error handling in ActorStatusLabel and RunnerPoolError components with better type definitions and pattern matching
  • Create Actor Dialog: Fixed datacenter parameter to use undefined instead of empty string for inspector app type
  • Query Caching: Implemented a previousQueryCache to prevent redundant query invalidation and improve performance
  • Loading Component: Replaced PendingRouteLayout with FullscreenLoading for namespace routes

How to test?

  1. Verify the sidebar layout and buttons display correctly with proper sizing
  2. Test actor error handling by triggering various error conditions
  3. Create an actor in inspector mode and verify datacenter parameter is handled correctly
  4. Navigate between pages to ensure query caching works properly without redundant invalidations
  5. Check that namespace routes show the correct loading component

Why make this change?

These changes improve the user experience by:

  • Creating a more consistent and visually appealing sidebar layout
  • Providing better error handling and more descriptive error messages for actor operations
  • Optimizing query performance by preventing unnecessary invalidations
  • Using appropriate loading components for a smoother user experience during navigation

The code structure is now more maintainable with better type definitions and pattern matching for error handling.

@railway-app
Copy link

railway-app bot commented Jan 19, 2026

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

Service Status Web Updated (UTC)
frontend-inspector 😴 Sleeping (View Logs) Web Jan 20, 2026 at 7:06 pm
frontend-cloud 😴 Sleeping (View Logs) Web Jan 20, 2026 at 7:06 pm
website 😴 Sleeping (View Logs) Web Jan 20, 2026 at 7:02 pm

@railway-app railway-app bot temporarily deployed to rivet-frontend / rivet-pr-3978 January 19, 2026 21:10 Destroyed
Copy link
Contributor Author

jog1t commented Jan 19, 2026


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 19, 2026

More templates

@rivetkit/cloudflare-workers

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

@rivetkit/db

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

@rivetkit/framework-base

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

@rivetkit/next-js

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

@rivetkit/react

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

rivetkit

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

@rivetkit/sql-loader

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

@rivetkit/virtual-websocket

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

@rivetkit/engine-runner

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

@rivetkit/engine-runner-protocol

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

commit: c997179

@claude
Copy link

claude bot commented Jan 19, 2026

PR Review: fix(inspector): prevent error flickering

Summary

This PR addresses error flickering in the inspector by improving query caching, enhancing error handling with better TypeScript types, and refining the UI layout for the sidebar. The changes focus on preventing redundant query invalidations and providing more robust error pattern matching.


Code Quality & Best Practices

Strengths

  1. Improved Type Safety (actor-status-label.tsx)

    • The RunnerPoolError component now has explicit type definitions instead of object | string | undefined
    • Better pattern matching with ts-pattern that covers all error variants
  2. Better User Experience

    • Using undefined instead of empty string for datacenter parameter aligns with API expectations
    • Proper component selection (FullscreenLoading vs PendingRouteLayout) for namespace routes
  3. Layout Improvements

    • Wrapping sidebar links in a flex container provides better control and consistency
    • Consistent sizing with size="xs" across buttons

Critical Issues

1. Query Cache Memory Leak (global.ts:14-52)

The implementation of previousQueryCache has a critical memory leak. Queries are added to previousQueryCache on success but never removed (except on error). This cache will grow indefinitely as users navigate the app.

Recommended Fix:
Use a Set<string> to track query keys instead of a full QueryCache, and ensure proper cleanup when queries are removed from the main cache.

2. Unclear Intent & Documentation

The query caching logic lacks documentation explaining what "error flickering" means specifically and why this approach was chosen. Adding comments would help future maintainers understand the intent.


Minor Issues

3. Type Definition Could Be Stricter (actor-status-label.tsx:162-168)
The unknown type for serverless errors could be replaced with proper interfaces for better type safety.

4. Pattern Matching Complexity (actor-status-label.tsx:99-106)
The chained .or() pattern for runner_pool_error is hard to read. Consider using P.union() for better clarity.

5. Icon Spacing (layout.tsx:283-288)
The ms-1 class on icons within endIcon might create double spacing since the prop should handle spacing automatically.


Performance Considerations

Every successful status check invalidates ALL queries with errors. In a large app with many failed queries, this could cause performance issues. Consider limiting invalidation to related queries or adding throttling.


Test Coverage

⚠️ Missing Tests - No tests for the new query caching logic. Consider adding unit tests for the cache behavior and integration tests for error flickering scenarios.


Recommendations

Before Merge:

  1. MUST FIX: Address the memory leak in previousQueryCache
  2. SHOULD ADD: Add documentation/comments explaining the caching strategy
  3. SHOULD ADD: Add tests for the query caching behavior
  4. CONSIDER: Tighten TypeScript types for error objects
  5. CONSIDER: Simplify complex pattern matching expressions

Conclusion

The PR addresses a real UX issue and makes meaningful improvements to error handling and UI layout. However, the query caching implementation has a critical memory leak that must be fixed before merge.

Overall Assessment: ⚠️ Needs Changes - Fix memory leak before merging.


Review generated by Claude Code

@linear
Copy link

linear bot commented Jan 19, 2026

@jog1t jog1t marked this pull request as ready for review January 19, 2026 21:33
@graphite-app
Copy link
Contributor

graphite-app bot commented Jan 19, 2026

Graphite Automations

"Test" took an action on this PR • (01/19/26)

1 assignee was added to this PR based on Kacper Wojciechowski's automation.

@jog1t jog1t force-pushed the 01-19-fix_inspector_prevent_error_flickering branch from e01279b to c997179 Compare January 20, 2026 18:53
@railway-app railway-app bot temporarily deployed to rivet-frontend / rivet-pr-3978 January 20, 2026 18:53 Destroyed
@graphite-app
Copy link
Contributor

graphite-app bot commented Jan 20, 2026

Merge activity

  • Jan 20, 7:20 PM UTC: jog1t added this pull request to the Graphite merge queue.
  • Jan 20, 7:21 PM UTC: CI is running for this pull request on a draft pull request (#3984) due to your merge queue CI optimization settings.
  • Jan 20, 7:21 PM UTC: Merged by the Graphite merge queue via draft PR: #3984.

graphite-app bot pushed a commit that referenced this pull request Jan 20, 2026
Closes FRONT-915

### TL;DR

Improved UI components and error handling for the Rivet application, with focus on sidebar layout, actor error handling, and query caching.

### What changed?

- **Sidebar Layout**: Restructured the sidebar navigation with proper sizing and layout for buttons, wrapped links in a flex container, and added proper icon placement
- **Actor Error Handling**: Enhanced error handling in `ActorStatusLabel` and `RunnerPoolError` components with better type definitions and pattern matching
- **Create Actor Dialog**: Fixed datacenter parameter to use `undefined` instead of empty string for inspector app type
- **Query Caching**: Implemented a `previousQueryCache` to prevent redundant query invalidation and improve performance
- **Loading Component**: Replaced `PendingRouteLayout` with `FullscreenLoading` for namespace routes

### How to test?

1. Verify the sidebar layout and buttons display correctly with proper sizing
2. Test actor error handling by triggering various error conditions
3. Create an actor in inspector mode and verify datacenter parameter is handled correctly
4. Navigate between pages to ensure query caching works properly without redundant invalidations
5. Check that namespace routes show the correct loading component

### Why make this change?

These changes improve the user experience by:

- Creating a more consistent and visually appealing sidebar layout
- Providing better error handling and more descriptive error messages for actor operations
- Optimizing query performance by preventing unnecessary invalidations
- Using appropriate loading components for a smoother user experience during navigation

The code structure is now more maintainable with better type definitions and pattern matching for error handling.
@graphite-app graphite-app bot closed this Jan 20, 2026
@graphite-app graphite-app bot deleted the 01-19-fix_inspector_prevent_error_flickering branch January 20, 2026 19:21
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