Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughAdds client-side collapsing for sentinel nodes with auto-collapse on initial load, memoized visible-tree computation, unified node renderer, expanded node component props to support collapse and stacked views, and exports/constants for node sizing and collapse threshold. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant UI as DeploymentNetworkView
participant State as React State
participant Layout as TreeLayout
participant Sentinel as SentinelNode
User->>UI: click collapse toggle
UI->>State: toggleSentinel(sentinelId)
State->>State: update collapsedSentinelIds
State-->>UI: re-render
UI->>UI: compute visibleTree & sentinelChildrenMap (useMemo)
UI->>Layout: render with visibleTree & renderDeploymentNode
Layout->>Sentinel: render with isCollapsed & onToggleCollapse
Sentinel->>UI: emit onToggleCollapse when toggled
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan
Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@web/apps/dashboard/app/`(app)/[workspaceSlug]/projects/[projectId]/(overview)/deployments/[deploymentId]/network/deployment-network-view.tsx:
- Around line 116-123: The collapsed Sentinel render changes the DOM footprint
(sentinel-node.tsx adds a pill and the collapsed wrapper grows by frontOffset)
which mismatches TreeLayout's fixed NODE_SIZES and causes connector drift;
update the layout footprint rather than only the DOM by either (A) updating
NODE_SIZES in components/nodes/types.ts to include the collapsed frontOffset for
the sentinel node type or (B) ensure the collapsed SentinelNode render (used
where isSentinelNode(...) returns true and controlled by
collapsedSentinelIds/toggleSentinel) keeps the same bounding box as the expanded
state (for example by reserving the extra frontOffset space in the collapsed
wrapper via padding/margin or an invisible spacer) so TreeLayout sees a
consistent size for both collapsed and expanded states.
- Around line 94-98: The sentinel remains collapsed after scaling below the
collapse threshold because the collapse check only uses collapsedSentinelIds;
update the collapse condition in deployment-network-view.tsx so that you only
slice children when both collapsedSentinelIds.has(node.id) AND
node.metadata.instances > COLLAPSE_THRESHOLD (or alternatively remove node.id
from collapsedSentinelIds when instances <= COLLAPSE_THRESHOLD). Locate the
logic around isSentinelNode, map.set and the ternary returning { ...node,
children: node.children?.slice(0, 1) } and change it to include the
instance-count check (or perform the cleanup of collapsedSentinelIds) to ensure
sentinels auto-expand when instances drop below COLLAPSE_THRESHOLD; the toggle
in sentinel-node.tsx will then behave correctly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: add124e5-ddd1-478c-8aeb-d8b672103a33
📒 Files selected for processing (8)
web/apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/(overview)/deployments/[deploymentId]/network/deployment-network-view.tsxweb/apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/(overview)/deployments/[deploymentId]/network/unkey-flow/components/nodes/index.tsweb/apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/(overview)/deployments/[deploymentId]/network/unkey-flow/components/nodes/instance-node.tsxweb/apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/(overview)/deployments/[deploymentId]/network/unkey-flow/components/nodes/node-wrapper/health-banner.tsxweb/apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/(overview)/deployments/[deploymentId]/network/unkey-flow/components/nodes/node-wrapper/node-wrapper.tsxweb/apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/(overview)/deployments/[deploymentId]/network/unkey-flow/components/nodes/sentinel-node.tsxweb/apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/(overview)/deployments/[deploymentId]/network/unkey-flow/components/nodes/skeleton-node/skeleton-node.tsxweb/apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/(overview)/deployments/[deploymentId]/network/unkey-flow/components/nodes/types.ts
...ojects/[projectId]/(overview)/deployments/[deploymentId]/network/deployment-network-view.tsx
Show resolved
Hide resolved
...ojects/[projectId]/(overview)/deployments/[deploymentId]/network/deployment-network-view.tsx
Show resolved
Hide resolved
mcstepp
left a comment
There was a problem hiding this comment.
The rabbit issue about if instances scale down below threshold for collapse while collapsed is legit and should be fixed
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@web/apps/dashboard/app/`(app)/[workspaceSlug]/projects/[projectId]/(overview)/deployments/[deploymentId]/network/deployment-network-view.tsx:
- Around line 66-85: The auto-collapse uses the live child nodes count via
isInstanceNode and COLLAPSE_THRESHOLD but the collapse logic compares
node.metadata.instances, causing divergence; update the collapse routine (the
function that currently reads node.metadata.instances) to compute the instance
count the same way as the useEffect—e.g., derive instanceChildrenCount from
(node.children ?? []).filter(isInstanceNode).length or extract into a shared
helper like getInstanceCount(node) and use that from both the useEffect and the
collapse function, then compare against COLLAPSE_THRESHOLD and call
setCollapsedSentinelIds as before.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: b2ecdc86-efc7-41ac-aebe-ee99179147c2
📒 Files selected for processing (1)
web/apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/(overview)/deployments/[deploymentId]/network/deployment-network-view.tsx
| useEffect(() => { | ||
| if (hasAutoCollapsed.current || !defaultTree) { | ||
| return; | ||
| } | ||
| hasAutoCollapsed.current = true; | ||
| const toCollapse = new Set<string>(); | ||
| if (isOriginNode(defaultTree)) { | ||
| for (const child of defaultTree.children ?? []) { | ||
| if ( | ||
| isSentinelNode(child) && | ||
| (child.children ?? []).filter(isInstanceNode).length > COLLAPSE_THRESHOLD | ||
| ) { | ||
| toCollapse.add(child.id); | ||
| } | ||
| } | ||
| } | ||
| if (toCollapse.size > 0) { | ||
| setCollapsedSentinelIds(toCollapse); | ||
| } | ||
| }, [defaultTree]); |
There was a problem hiding this comment.
Minor inconsistency in threshold check between auto-collapse and collapse logic.
Line 76 counts actual instance children via filter(isInstanceNode).length, but line 98 in the collapse function uses node.metadata.instances. If these values ever diverge (e.g., partially loaded children), behavior could be inconsistent—a sentinel might auto-collapse but not stay collapsed, or vice versa.
Consider using the same source of truth in both places for predictability.
💡 Suggested alignment
Use instanceChildren.length instead of node.metadata.instances in the collapse function (line 98):
- return collapsedSentinelIds.has(node.id) && node.metadata.instances > COLLAPSE_THRESHOLD
+ return collapsedSentinelIds.has(node.id) && instanceChildren.length > COLLAPSE_THRESHOLD🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@web/apps/dashboard/app/`(app)/[workspaceSlug]/projects/[projectId]/(overview)/deployments/[deploymentId]/network/deployment-network-view.tsx
around lines 66 - 85, The auto-collapse uses the live child nodes count via
isInstanceNode and COLLAPSE_THRESHOLD but the collapse logic compares
node.metadata.instances, causing divergence; update the collapse routine (the
function that currently reads node.metadata.instances) to compute the instance
count the same way as the useEffect—e.g., derive instanceChildrenCount from
(node.children ?? []).filter(isInstanceNode).length or extract into a shared
helper like getInstanceCount(node) and use that from both the useEffect and the
collapse function, then compare against COLLAPSE_THRESHOLD and call
setCollapsedSentinelIds as before.
What does this PR do?
Screen.Recording.2026-03-16.at.12.30.58.mov