Skip to content

chore: actors mockup#4627

Draft
NathanFlurry wants to merge 1 commit intomainfrom
04-12-chore_actors_mockup
Draft

chore: actors mockup#4627
NathanFlurry wants to merge 1 commit intomainfrom
04-12-chore_actors_mockup

Conversation

@NathanFlurry
Copy link
Copy Markdown
Member

Description

Please include a summary of the changes and the related issue. Please also include relevant motivation and context.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

How Has This Been Tested?

Please describe the tests that you ran to verify your changes.

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

Copy link
Copy Markdown
Member Author

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

@railway-app
Copy link
Copy Markdown

railway-app bot commented Apr 13, 2026

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

Service Status Web Updated (UTC)
website 😴 Sleeping (View Logs) Web Apr 13, 2026 at 2:39 am
frontend-cloud 😴 Sleeping (View Logs) Web Apr 13, 2026 at 2:36 am
frontend-inspector 😴 Sleeping (View Logs) Web Apr 13, 2026 at 2:34 am
mcp-hub ✅ Success (View Logs) Web Apr 13, 2026 at 2:27 am
ladle ❌ Build Failed (View Logs) Web Apr 13, 2026 at 2:27 am
kitchen-sink ❌ Build Failed (View Logs) Web Apr 13, 2026 at 2:27 am

@claude
Copy link
Copy Markdown

claude bot commented Apr 13, 2026

PR Review: chore: actors mockup

This is a draft UI prototype for the actors dashboard — the review below is scoped to the mockup's goals, not production-readiness.


Overview

A clean, self-contained prototype. The ReactFlow canvas, sliding popovers, and animation approach all look reasonable for a mockup. A few technical issues worth flagging even at this stage.


Bugs / Issues

loadCanvasState called twice on mount (layout-mockup.tsx)

The function reads from localStorage twice during initialization — once for the nodes state initializer and once for the edges initializer. These should share a single call:

const [nodes, setNodes] = useState<Node<ActorNodeData>[]>(() => {
  const saved = loadCanvasState();
  if (saved?.nodes?.length) return saved.nodes;
  return makeDefaultNodes();
});
const [edges, setEdges] = useState<Edge[]>(() => {
  const saved = loadCanvasState(); // second read — redundant
  return saved?.edges ?? [];
});

Real actors overwrite saved positions every reload

The useEffect that maps real actors to nodes always resets positions to a grid, ignoring any positions previously saved to localStorage. If the canvas is ever connected to real data, arranged layouts will be lost on every reload.

ActorDetailPlaceholder tabs pattern is unusual

The component manages the active tab via both React state and the Tabs value prop, then manually hides the code editor with a hidden class. The second TabsContent uses value={inspectorTab} which changes its value prop on every tab switch, causing unnecessary remounting. Consider using standard TabsContent per tab, or a single controlled render outside of TabsContent.

rightMounted is never reset after close

After the right panel slides out, rightMounted stays true and the panel remains in the DOM permanently. For a mockup this is fine, but if the panel holds a real CodeMirror instance, it will never be cleaned up.


Minor

  • proOptions={{ hideAttribution: true }}: Hiding the ReactFlow attribution requires a Pro license. Fine for internal prototyping, but should not ship to production without verifying the license covers this use.

  • localStorage key is generic: "mockup-canvas-state" is unnamespaced. If the key format ever changes, old saved state will be silently parsed with unexpected shape. A prefixed key ("rivet:mockup-canvas-state") would make it easier to bust the cache intentionally.

  • deleteKeyCode="Backspace" on the ReactFlow canvas: Backspace will delete selected nodes/edges when the canvas is focused. This may surprise users — consider Delete or leaving it unset for a mockup.

  • AgentChat input has no handler: The Send button and input are non-functional, which is expected for a mockup, but the input is missing an accessible label (htmlFor/aria-label).

  • EXAMPLE_CODE uses override initialize() but the class body uses this.state.scores without initializing the type. The code shown in the Code tab is presented as a live example — worth making sure it matches the current RivetKit API so it does not mislead.


Suggestions for the Real Implementation

  • The actor list in the right panel (ActorListPlaceholder) and the canvas nodes are completely decoupled — selecting a canvas node does not highlight the corresponding list entry and vice versa. A shared selection model (e.g., lifted state or a context) will be needed.
  • The fake AI chat (AgentChat) hardcodes a conversation. If this feature goes forward, the messages should stream from a real endpoint rather than being static — even during prototyping, a mock streaming hook would give a better feel for the intended UX.

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.

1 participant