Conversation
|
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughAdds a new OpenAPI Diff feature with a dedicated page, deployment selectors, and a client-side diff viewer. Integrates a compact OpenAPI diff section into project details. Refactors StatusIndicator to accept status and show tooltips. Adds navigation tab and context field. Removes legacy diff pages/components. Minor UI library tooltip/select adjustments. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor U as User
participant P as OpenAPI Diff Page
participant LQ as Live Query (deployments)
participant S as DeploymentSelect
participant Q as TRPC getOpenApiDiff
participant V as DiffViewerContent
U->>P: Navigate to /projects/:id/openapi-diff
P->>LQ: Fetch deployments (latest first)
LQ-->>P: Deployments list
P->>S: Render From/To selectors (with disabled conflicts)
U->>S: Choose From and To deployments
S-->>P: onValueChange(selected IDs)
alt Both selections present
P->>Q: Query diff({ fromId, toId })
Q-->>P: Diff data or error
opt Loading
P-->>U: Show loading state
end
alt Diff data
P->>V: Render diff content (filters, groups)
V-->>U: Interactive diff UI
else Error
P-->>U: Show error message
end
else Missing selection
P-->>U: Show empty state
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
ogzhanolguncu
requested review from
Flo4604,
MichaelUnkey,
chronark,
mcstepp and
perkinsjr
as code owners
|
Thank you for following the naming conventions for pull request titles! 🙏 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (17)
apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/details/active-deployment-card/status-indicator.tsx(1 hunks)apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/details/project-details-expandables/detail-section.tsx(1 hunks)apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/details/project-details-expandables/sections.tsx(2 hunks)apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/details/project-details-expandables/sections/open-api-diff.tsx(1 hunks)apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/diff/[...compare]/components/client.tsx(0 hunks)apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/diff/[...compare]/page.tsx(0 hunks)apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/diff/page.tsx(0 hunks)apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/layout-provider.tsx(1 hunks)apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/layout.tsx(1 hunks)apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/navigations/project-sub-navigation.tsx(3 hunks)apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/openapi-diff/components/client.tsx(1 hunks)apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/openapi-diff/components/empty.tsx(1 hunks)apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/openapi-diff/deployment-select.tsx(1 hunks)apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/openapi-diff/page.tsx(1 hunks)apps/dashboard/lib/trpc/routers/deploy/deployment/getOpenApiDiff.ts(0 hunks)internal/ui/src/components/form/select.tsx(1 hunks)internal/ui/src/components/tooltip.tsx(1 hunks)
💤 Files with no reviewable changes (4)
- apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/diff/[...compare]/components/client.tsx
- apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/diff/[...compare]/page.tsx
- apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/diff/page.tsx
- apps/dashboard/lib/trpc/routers/deploy/deployment/getOpenApiDiff.ts
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-07-28T20:38:53.244Z
Learnt from: mcstepp
PR: unkeyed/unkey#3662
File: apps/dashboard/app/(app)/projects/[projectId]/diff/[...compare]/components/client.tsx:322-341
Timestamp: 2025-07-28T20:38:53.244Z
Learning: In apps/dashboard/app/(app)/projects/[projectId]/diff/[...compare]/components/client.tsx, mcstepp prefers to keep hardcoded endpoint logic in the getDiffType function during POC phases for demonstrating diff functionality, rather than implementing a generic diff algorithm. This follows the pattern of keeping simplified implementations for demonstration purposes.
Applied to files:
apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/openapi-diff/components/client.tsxapps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/openapi-diff/components/empty.tsxapps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/openapi-diff/page.tsxapps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/details/project-details-expandables/sections/open-api-diff.tsx
📚 Learning: 2025-07-25T19:11:00.208Z
Learnt from: mcstepp
PR: unkeyed/unkey#3662
File: apps/dashboard/lib/trpc/routers/deployment/getOpenApiDiff.ts:110-147
Timestamp: 2025-07-25T19:11:00.208Z
Learning: In apps/dashboard/lib/trpc/routers/deployment/getOpenApiDiff.ts, the user mcstepp prefers to keep mock data fallbacks in POC/demonstration code for simplicity, even if it wouldn't be production-ready. This aligns with the PR being work-in-progress for demonstration purposes.
Applied to files:
apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/details/project-details-expandables/sections/open-api-diff.tsx
🧬 Code graph analysis (6)
apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/details/project-details-expandables/sections.tsx (1)
apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/details/project-details-expandables/sections/open-api-diff.tsx (1)
OpenApiDiff(26-108)
apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/openapi-diff/components/client.tsx (5)
internal/icons/src/icons/triangle-warning.tsx (1)
TriangleWarning(16-55)internal/icons/src/icons/circle-warning.tsx (1)
CircleWarning(15-42)internal/icons/src/icons/circle-xmark.tsx (1)
CircleXMark(16-62)internal/icons/src/icons/chevron-down.tsx (1)
ChevronDown(15-35)apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/diff/[...compare]/components/client.tsx (1)
changelog(32-986)
apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/navigations/project-sub-navigation.tsx (1)
internal/icons/src/icons/connections.tsx (1)
Connections(15-54)
apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/openapi-diff/deployment-select.tsx (1)
internal/db/src/schema/deployments.ts (1)
deployments(9-54)
apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/openapi-diff/page.tsx (5)
apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/layout-provider.tsx (1)
useProjectLayout(16-22)internal/db/src/schema/deployments.ts (1)
deployments(9-54)apps/dashboard/lib/trpc/server.ts (1)
trpc(7-14)apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/openapi-diff/deployment-select.tsx (1)
DeploymentSelect(19-85)apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/openapi-diff/components/client.tsx (1)
DiffViewerContent(31-315)
apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/details/project-details-expandables/sections/open-api-diff.tsx (5)
apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/details/active-deployment-card/status-indicator.tsx (2)
DiffStatus(5-5)StatusIndicator(26-87)apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/layout-provider.tsx (1)
useProjectLayout(16-22)apps/dashboard/lib/trpc/server.ts (1)
trpc(7-14)apps/dashboard/lib/shorten-id.ts (1)
shortenId(5-54)internal/icons/src/icons/arrow-right.tsx (1)
ArrowRight(15-48)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Test Go API Local / Test
- GitHub Check: Test Packages / Test
- GitHub Check: Build / Build
- GitHub Check: Test API / API Test Local
🔇 Additional comments (31)
internal/ui/src/components/tooltip.tsx (1)
18-28: LGTM! Portal wrapping follows Radix UI best practices.Wrapping
TooltipContentwithTooltipPrimitive.Portalensures the tooltip renders at the document root, preventing z-index and overflow issues. This is the recommended pattern for Radix UI tooltips.apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/layout-provider.tsx (1)
7-9: LGTM! Context type extended correctly.The
liveDeploymentIdfield is properly typed as optional and nullable, aligning with the value provided inlayout.tsx. The optional nature ensures backward compatibility.apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/openapi-diff/components/empty.tsx (1)
1-31: LGTM! Well-crafted empty state component.The component provides clear guidance with appropriate visual hierarchy and smooth animations. The inline
animationDurationstyle on line 17 is acceptable for customizing the pulse animation timing.apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/layout.tsx (3)
37-37: LGTM! Deployment ID extraction is correct.The use of
.at(0)safely handles empty arrays, returningundefinedwhen no project is found, which is compatible with the context type.
61-61: LGTM! Context value properly includes deployment ID.The
liveDeploymentIdis correctly passed to the context provider, making it available to all consumers viauseProjectLayout().
39-45: Remove suppressions for collection.utils.refetch()TanStack DB v0.1.x officially supports
collection.utils.refetch()and auto-refetch handlers, so the@ts-expect-errorandbiome-ignoreare no longer needed. Remove those suppressions and update types or dependencies if TS still complains. cite12Likely an incorrect or invalid review comment.
apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/details/project-details-expandables/detail-section.tsx (3)
5-6: LGTM! Nullable props enable flexible layouts.Making
iconandlabelnullable allows for varied detail row configurations while maintaining type safety.
14-21: LGTM! Efficient early return for null cases.The early return when both
iconandlabelare absent allows the children to occupy the full width without unnecessary wrapper divs, improving layout flexibility.
26-31: LGTM! Conditional rendering prevents empty containers.The conditional blocks ensure that icon and label wrappers only render when their respective values are present, avoiding unnecessary DOM elements.
apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/navigations/project-sub-navigation.tsx (1)
5-5: LGTM! Clean navigation extension.The new "openapi-diff" tab is properly integrated with the existing navigation structure, following the same pattern as other tabs.
Also applies to: 50-50, 76-81
apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/details/project-details-expandables/sections.tsx (2)
24-25: Type expansion enables flexible rendering.Widening
iconandlabelto acceptnullallows the new OpenAPI changes section to render without header elements.
38-48: LGTM! OpenAPI changes section integrated cleanly.The new section is positioned prominently at the top and uses
alignment: "start"for proper content layout.apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/openapi-diff/deployment-select.tsx (2)
19-84: LGTM! Comprehensive deployment selector.The component handles all necessary states (loading, empty, disabled) and provides good UX with formatted dates, truncated messages, and tooltips for disabled items.
45-52: VerifyDeployment.idnull-safetyWe couldn’t locate the
Deploymenttype to confirm thatidis always defined. Please verify thatDeployment.idis non-optional in your schema; if it can be absent, guard thesubstringcall (e.g.deployment.id?.substring(0, 7)).apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/details/project-details-expandables/sections/open-api-diff.tsx (1)
52-66: LGTM! Edge cases handled appropriately.The component gracefully handles the single-deployment scenario and the no-deployment case, providing appropriate UI for each state.
Also applies to: 68-107
apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/openapi-diff/components/client.tsx (7)
36-51: LGTM! Efficient state management with memoization.The component uses
useStatefor local UI state anduseMemofor derived statistics, ensuring optimal re-render performance.
53-85: LGTM! Well-structured filtering and grouping logic.The cascading memoization (filteredChanges → groupedChanges) ensures efficient updates and the grouping structure aligns well with OpenAPI endpoint organization.
93-111: LGTM! Consistent severity mapping.The helper functions provide consistent icon and color mappings for all severity levels (breaking/warning/info).
113-121: LGTM! Appropriate empty state.The early return for empty changelogs provides clear user feedback with contextual deployment labels.
126-151: LGTM! Clear and informative summary header.The stats display provides quick insights into the nature and scope of changes with appropriate badges for breaking changes and warnings.
154-219: LGTM! Comprehensive and accessible filter controls.The filter UI provides search, level selection, and operation filtering with a clear button to reset. The clear icon in the search input is a nice UX touch.
222-312: LGTM! Well-structured collapsible change groups.The nested structure (path → operation → changes) with expand/collapse functionality makes it easy to navigate large diffs. The animation and color coding enhance the user experience.
apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/details/active-deployment-card/status-indicator.tsx (2)
5-11: LGTM! Clean type definitions.The
DiffStatustype andStatusIndicatorPropsinterface are well-defined, with sensible defaults for optional properties.
13-24: LGTM! Exhaustive tooltip content mapping.The helper function provides clear, user-friendly tooltip messages for all status values.
apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/openapi-diff/page.tsx (7)
1-14: LGTM! Clean imports.All necessary dependencies are imported for the diff page functionality.
15-21: LGTM! Proper state initialization.Context and URL params are extracted correctly, and deployment state is initialized appropriately.
22-32: LGTM! Well-structured live query.The deployments query correctly joins with environments, orders chronologically, and has a reasonable limit.
34-64: LGTM! Robust URL parameter handling.The effect correctly prioritizes URL params, validates deployment existence, and falls back to live deployment appropriately.
66-78: LGTM! Properly gated TRPC query.The diff query is correctly enabled only when both deployments are selected, preventing unnecessary API calls.
80-95: LGTM! Clean deployment label formatter.The helper correctly formats deployment labels with branch and short SHA, with appropriate fallbacks.
97-214: LGTM! Well-structured render logic with proper state handling.The component correctly handles:
- Mutual exclusion between deployment selectors
- Empty state when no deployments selected
- Loading state during diff analysis
- Error state with clear messaging
- Content display with proper labels
The conditional rendering logic is clean and all edge cases are covered.
| // biome-ignore lint/suspicious/noArrayIndexKey: <explanation> | ||
| key={index} |
There was a problem hiding this comment.
Replace placeholder biome-ignore explanation.
The ignore comment has a placeholder <explanation> instead of a meaningful justification.
Apply this diff:
- <div
- // biome-ignore lint/suspicious/noArrayIndexKey: <explanation>
- key={index}
+ <div
+ // biome-ignore lint/suspicious/noArrayIndexKey: Static animation array with stable indices
+ key={index}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // biome-ignore lint/suspicious/noArrayIndexKey: <explanation> | |
| key={index} | |
| <div | |
| // biome-ignore lint/suspicious/noArrayIndexKey: Static animation array with stable indices | |
| key={index} |
🤖 Prompt for AI Agents
In
apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/details/active-deployment-card/status-indicator.tsx
around lines 65-66, the biome-ignore comment currently contains a placeholder
"<explanation>"; replace it with a concise, factual justification explaining why
using the array index as the React key is acceptable here (for example: "items
are static/never reordered/uniquely scoped to this render so index stability is
guaranteed"), or alternatively remove the ignore and refactor to use a stable
unique id for the key; ensure the justification mentions the specific constraint
(static list, no reordering, or key derived elsewhere) so the lint suppression
is meaningful.
| const diff = trpc.deploy.deployment.getOpenApiDiff.useQuery({ | ||
| newDeploymentId: newDeployment?.id ?? "", | ||
| oldDeploymentId: oldDeployment?.id ?? "", | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
Verify TRPC query behavior with empty deployment IDs.
When newDeployment or oldDeployment are undefined, the query receives empty strings. Ensure the backend gracefully handles this case or add a guard to skip the query.
🏁 Script executed:
#!/bin/bash
# Check the getOpenApiDiff implementation to verify empty string handling
ast-grep --pattern 'getOpenApiDiff$$$'
rg -nP --type=ts -C5 'getOpenApiDiff' apps/dashboard/lib/trpc/routers/deployment/Length of output: 83
🏁 Script executed:
#!/bin/bash
# locate implementation file for getOpenApiDiff
files=$(rg -l "getOpenApiDiff" -g '*.ts' apps/dashboard/lib/trpc/routers || true)
echo "Found files:"
echo "$files"
# inspect occurrences in each file
for file in $files; do
echo "--- File: $file ---"
rg -n -C5 "getOpenApiDiff" "$file"
doneLength of output: 1945
Guard the TRPC query from running with empty IDs
The router’s input schema accepts empty strings and there’s no check in the procedure, so calling it with missing IDs can error. Wrap the useQuery in enabled: !!newDeployment?.id && !!oldDeployment?.id (or skip the hook when either ID is empty) [apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/details/project-details-expandables/sections/open-api-diff.tsx:44-47]
🤖 Prompt for AI Agents
In
apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/details/project-details-expandables/sections/open-api-diff.tsx
around lines 44 to 47, the TRPC useQuery is called with possibly empty
deployment IDs which can cause the procedure to error; guard the query by adding
an enabled condition that only runs the query when both IDs are non-empty (e.g.
pass enabled: !!newDeployment?.id && !!oldDeployment?.id) or skip calling
useQuery entirely when either ID is missing so the hook does not execute with
empty strings.
| // @ts-expect-error I have no idea why this whines about type diff | ||
| const status = getDiffStatus(diff.data); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Investigate and document the type mismatch.
The @ts-expect-error comment indicates uncertainty about the type issue. This should be investigated and either fixed or documented with a clear explanation of why the type mismatch is expected.
If the types genuinely conflict and there's a valid reason, update the comment to be more specific:
- // @ts-expect-error I have no idea why this whines about type diff
+ // @ts-expect-error GetOpenApiDiffResponse type doesn't match getDiffStatus parameter due to [specific reason]
const status = getDiffStatus(diff.data);Or investigate and fix the underlying type issue.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // @ts-expect-error I have no idea why this whines about type diff | |
| const status = getDiffStatus(diff.data); | |
| // @ts-expect-error GetOpenApiDiffResponse type doesn't match getDiffStatus parameter due to [specific reason] | |
| const status = getDiffStatus(diff.data); |
| className={cn( | ||
| "relative flex w-full cursor-default select-none items-center rounded-md py-1.5 pl-2 pr-8 text-[13px] outline-none", | ||
| "text-gray-12 hover:bg-gray-4 focus:bg-gray-5 data-[disabled]:opacity-50 data-[disabled]:pointer-events-none", | ||
| "text-gray-12 hover:bg-gray-4 focus:bg-gray-5 data-[disabled]:opacity-50", | ||
| className, | ||
| )} |
There was a problem hiding this comment.
Restore disabled-item interaction guard.
Dropping data-[disabled]:pointer-events-none means disabled options now pick up the existing hover:bg-gray-4/focus:bg-gray-5 styles, so they highlight and behave like active options even though they stay non-selectable. Please keep the pointer-events guard (or explicitly noop the hover/focus states) while adding the opacity fade, e.g. data-[disabled]:pointer-events-none data-[disabled]:opacity-50.
🤖 Prompt for AI Agents
In internal/ui/src/components/form/select.tsx around lines 139 to 143, disabled
items lost the pointer-events guard and now still receive hover/focus styles;
restore the interaction guard and keep the opacity change by adding the Tailwind
data attribute for pointer events to the class list (e.g. include
data-[disabled]:pointer-events-none alongside data-[disabled]:opacity-50) so
disabled options don’t respond to hover/focus while still showing the faded
appearance.
2 participants
What does this PR do?
This PR;
cursor-events-nonethingy from so we can assign sometitleor for accessibility and better UX.Fixes # (issue)
If there is not an issue for this, please create one first. This is used to tracking purposes and also helps use understand why this PR exists
Type of change
How should this be tested?
Checklist
Required
pnpm buildpnpm fmtconsole.logsgit pull origin mainAppreciated
Screen.Recording.2025-10-01.at.14.31.03.mov