Open
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds client-side sentinel collapse state with one-time auto-collapse, memoized visible-tree and sentinelChildrenMap, stacked-instance rendering for collapsed sentinels, a unified renderDeploymentNode callback, and new node sizing/collapse constants plus updated node component APIs. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant UI as DeploymentNetworkView
participant State as ReactState
participant Layout as TreeLayout
participant Sentinel as SentinelNode
User->>UI: click collapse toggle
UI->>State: toggleSentinel(sentinelId)
State->>State: update collapsedSentinelIds / hasAutoCollapsed
State-->>UI: re-render
UI->>UI: compute visibleTree & sentinelChildrenMap (useMemo)
UI->>Layout: render with visibleTree & renderDeploymentNode
Layout->>Sentinel: render with isCollapsed & onToggleCollapse
Sentinel->>UI: invoke onToggleCollapse when toggled
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 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)
Comment |
ogzhanolguncu
requested review from
Flo4604,
MichaelUnkey,
chronark,
imeyer,
mcstepp and
perkinsjr
as code owners
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
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
Outdated
Show resolved
Hide resolved
...ojects/[projectId]/(overview)/deployments/[deploymentId]/network/deployment-network-view.tsx
Show resolved
Hide resolved
mcstepp
requested changes
Mar 16, 2026
Collaborator
mcstepp
left a comment
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
Contributor
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
...ojects/[projectId]/(overview)/deployments/[deploymentId]/network/deployment-network-view.tsx
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
web/apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/(overview)/deployments/[deploymentId]/network/deployment-network-view.tsx (1)
123-124:⚠️ Potential issue | 🟠 MajorKeep collapsed-state checks consistent across sentinel and instance render paths.
Line 98 correctly gates collapse by threshold, but Lines 123 and 133 still use ID-only collapse state. If a sentinel was collapsed at 4 instances and later drops to 3, this can still force stacked rendering for each visible instance node.
💡 Minimal fix
if (isSentinelNode(node)) { + const instanceCount = sentinelChildrenMap.get(node.id)?.length ?? 0; + const isCollapsed = + collapsedSentinelIds.has(node.id) && instanceCount > COLLAPSE_THRESHOLD; return ( <SentinelNode node={node} deploymentId={deployment.id} - isCollapsed={collapsedSentinelIds.has(node.id)} + isCollapsed={isCollapsed} onToggleCollapse={isShowingSkeleton ? undefined : () => toggleSentinel(node.id)} /> ); } if (isInstanceNode(node)) { if (!parent || !isSentinelNode(parent)) { throw new Error("Instance node requires parent sentinel"); } - if (collapsedSentinelIds.has(parent.id)) { - const instances = sentinelChildrenMap.get(parent.id) ?? []; + const instances = sentinelChildrenMap.get(parent.id) ?? []; + const isParentCollapsed = + collapsedSentinelIds.has(parent.id) && instances.length > COLLAPSE_THRESHOLD; + if (isParentCollapsed) { const totalLayers = instances.length; const step = 10;Also applies to: 133-137
🤖 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 123 - 124, The sentinel render is using collapsedSentinelIds.has(node.id) directly for isCollapsed/onToggleCollapse, causing inconsistent behavior vs. the threshold-gated check used earlier; update the sentinel and instance render paths (the places setting isCollapsed and onToggleCollapse where you currently call collapsedSentinelIds.has(node.id)) to use the same collapse gating logic as line 98 (i.e., combine collapsedSentinelIds.has(node.id) with the collapse threshold check used there), and keep the onToggleCollapse hook as isShowingSkeleton ? undefined : () => toggleSentinel(node.id) so toggling remains disabled during skeleton view.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In
`@web/apps/dashboard/app/`(app)/[workspaceSlug]/projects/[projectId]/(overview)/deployments/[deploymentId]/network/deployment-network-view.tsx:
- Around line 123-124: The sentinel render is using
collapsedSentinelIds.has(node.id) directly for isCollapsed/onToggleCollapse,
causing inconsistent behavior vs. the threshold-gated check used earlier; update
the sentinel and instance render paths (the places setting isCollapsed and
onToggleCollapse where you currently call collapsedSentinelIds.has(node.id)) to
use the same collapse gating logic as line 98 (i.e., combine
collapsedSentinelIds.has(node.id) with the collapse threshold check used there),
and keep the onToggleCollapse hook as isShowingSkeleton ? undefined : () =>
toggleSentinel(node.id) so toggling remains disabled during skeleton view.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: bd57f60a-6a69-447a-aa5f-03ac55fefaeb
📒 Files selected for processing (1)
web/apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/(overview)/deployments/[deploymentId]/network/deployment-network-view.tsx
Collaborator
|
I didn't see the issue resolved or the changes? |
Contributor
Author
This should prevent it I guess |
perkinsjr
requested changes
Mar 19, 2026
...ojects/[projectId]/(overview)/deployments/[deploymentId]/network/deployment-network-view.tsx
Outdated
Show resolved
Hide resolved
Contributor
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 63-74: The effect defined in useEffect reads defaultTree but lists
currentTree in its dependency array, which is misleading and brittle; update the
dependency array to include defaultTree (or replace currentTree with
defaultTree) so the effect re-runs when defaultTree changes, and ensure
hasAutoCollapsed, computeAutoCollapsedSentinels, and setCollapsedSentinelIds
usage remains unchanged.
- Line 79: The code unsafely casts currentTree to OriginNodeType using "as
OriginNodeType", which breaks the guideline; replace the cast with a runtime
type guard that checks whether currentTree is an OriginNodeType (and not
SKELETON_TREE / SkeletonNode) before assigning to originNode: implement or call
an isOriginNode(obj): obj is OriginNodeType predicate (or inline checks for
distinguishing properties) and branch accordingly (e.g., handle skeleton case or
return early) so originNode is only set when the guard passes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 70c1d1ca-c1a0-4041-bd5c-4a8e4571fa29
📒 Files selected for processing (2)
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.ts
✅ Files skipped from review due to trivial changes (1)
- web/apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/(overview)/deployments/[deploymentId]/network/unkey-flow/components/nodes/index.ts
...ojects/[projectId]/(overview)/deployments/[deploymentId]/network/deployment-network-view.tsx
Outdated
Show resolved
Hide resolved
...ojects/[projectId]/(overview)/deployments/[deploymentId]/network/deployment-network-view.tsx
Show resolved
Hide resolved
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
Screen.Recording.2026-03-16.at.12.30.58.mov