chore(upstream): PR6 v2 review tab first pass (#3463)#182
Conversation
…uperset-sh#3463) * feat(desktop): add Review tab to v2 workspace sidebar Port the v1 Review tab functionality into the v2 workspace sidebar, replacing the "Checks — Coming soon" stub. Uses v2-native host-service endpoints (git.getPullRequest, git.getPullRequestThreads) instead of v1 electron endpoints. Features: - PR header with title, state icon, review decision badge - Collapsible CI checks section with status icons and links - PR comments with avatars, timestamps, and preview text - Copy individual comments or copy all to clipboard - Comment pane: click a comment to view rendered markdown in a tab - GitHub link in pane header to open comment on GitHub - Copyable tables in rendered markdown * fix(desktop): address review tab PR feedback - Fix memory leaks: add timeout cleanup refs in CommentPane and CopyableTable, clear previous timeout on rapid clicks - Add error state: show "Unable to load review status" when tRPC query fails instead of infinite loading spinner - Fix getIcon fallback: show MessageSquare icon when avatarUrl is missing instead of rendering <img src={undefined}> - Add aria-label to comment open button for screen readers - Add aria-hidden to decorative arrow icon in PRHeader - Add group-focus-visible:opacity-100 to PRHeader arrow for keyboard nav * fix(desktop): add .catch() to all clipboard promise chains Prevents unhandled promise rejections in the renderer if electronTrpcClient.external.copyText.mutate() fails. * fix(desktop): guard state updates after unmount in CommentPane Add isMountedRef to both CommentPane and CopyableTable to prevent setCopied calls after unmount when the async clipboard IPC resolves after the component is gone. * fix(desktop): fix comments not loading until refresh The threads query had staleTime: 30_000 which cached empty results when the query fired before the host-service had PR data synced. Remove staleTime so every mount/refocus fetches fresh data (background polling at 30s still runs via refetchInterval). Also tighten the enabled condition to use prQuery.isSuccess instead of !!prQuery.data for clearer intent, and use composite key for CheckRow to handle duplicate check names. * fix(desktop): unmount guard in CommentsSection + single-pass check status - Add isMountedRef to CommentsSection so markCopied doesn't set state after unmount (same pattern as CommentPane/CopyableTable) - Rewrite computeChecksStatus as single loop instead of 3 passes (filter + some(failure) + some(pending) each called coerceCheckStatus) * feat(desktop): render mermaid diagrams + syntax highlighting in comment pane Use the existing CodeBlock component which handles mermaid via @streamdown/mermaid and syntax highlighting via react-syntax-highlighter. * fix(desktop): lighten mermaid diagram background in comment pane * fix(desktop): hide mermaid label + lighten actual diagram background - Hide the "mermaid" label and action buttons on the block wrapper - Strip the outer border/padding so it blends with the page - Target the SVG element itself to lighten its background color * fix(desktop): revert hacky mermaid overrides, use CSS text color instead Revert the custom Streamdown/SyntaxHighlighter approach. Go back to using the shared CodeBlock component. Instead of changing the mermaid canvas background, override the SVG text fill and node/edge label colors via CSS to use --foreground, making them readable on both light and dark themes. * fix(desktop): use mermaid themeVariables for light text in dark mode Use Streamdown directly with custom themeVariables to set light text colors (primaryTextColor, nodeTextColor, labelTextColor, etc.) on the dark mermaid theme. Removes the CSS hacks that couldn't override mermaid's inline SVG styles. * fix(desktop): mermaid dark theme contrast + hide label/wrapper chrome - Use theme: "base" with full dark color palette so themeVariables actually control node fills (dark gray nodes, light text) - Hide "mermaid" label and action buttons via CSS - Strip background/border from outer block and inner wrapper so diagram sits directly on the page background * fix(desktop): extract CommentCodeBlock as standalone component Move code block rendering out of useMemo into a proper React component so the component identity is stable across renders. Prevents unmount/ remount of Streamdown/SyntaxHighlighter when the theme changes. Mermaid theme variable objects extracted as module-level constants.
Three functional issues Codex found on upstream superset-sh#3463 cherry-pick: 1. useReviewTab.tsx normalizeThreadsToComments only picked thread.comments[0], silently dropping every reply in a review thread. v1 flattened all comments per thread. Loop over thread.comments instead so the listing and badge match the real PR state. 2. CommentPane.tsx fed ReactMarkdown the raw comment body without overriding <a> or <img>. GitHub-supplied links would navigate the main window away, and arbitrary remote images would load. Add CommentLink (opens via electronTrpcClient.external.openUrl) and CommentImage (uses SafeImage, which only renders data: URLs). 3. useReviewTab.tsx returned badge: openCommentCount unconditionally, so the tab showed a "0" badge when there were no open comments. Changes tab already suppresses zero badges; match that. Deferred (upstream bug, host-service schema change required): - Review comments never get a GitHub URL because the GraphQL query and host-service types in packages/host-service don't fetch/expose reviewComment.url. Fixing this needs host-service router changes and is out of scope for this cherry-pick.
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 46 minutes and 21 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (20)
📝 WalkthroughWalkthroughA Review tab feature is added to the v2 workspace sidebar, replacing a "Coming soon" stub. The implementation includes a Changes
Sequence DiagramsequenceDiagram
participant User
participant ReviewTab as Review Tab
participant useReviewTab as useReviewTab Hook
participant tRPC as tRPC / Server
participant CommentPane as Comment Pane
User->>ReviewTab: View Review Tab
ReviewTab->>useReviewTab: Initialize hook
useReviewTab->>tRPC: Fetch PR metadata (getPullRequest)
tRPC-->>useReviewTab: PR + checks + status
useReviewTab->>tRPC: Fetch PR threads (getPullRequestThreads)
tRPC-->>useReviewTab: Comments + threads
useReviewTab->>ReviewTab: Normalized PR & comments
ReviewTab->>ReviewTab: Render PRHeader, ChecksSection, CommentsSection
User->>ReviewTab: Click on comment
ReviewTab->>ReviewTab: onOpenComment callback
ReviewTab->>CommentPane: Trigger pane creation/activation
CommentPane->>User: Display comment with Markdown + actions
User->>CommentPane: Click "Copy All"
CommentPane->>tRPC: copyText (external IPC)
tRPC-->>CommentPane: Text copied
CommentPane->>User: Show "Copied" feedback
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 531e704b80
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Codex review on #182: when getPullRequest transitions to null (e.g. PR closed/unlinked), React Query keeps the last successful getPullRequestThreads data in cache. The previous code normalized comments directly from that cached data without checking `pr`, so the Review tab kept showing stale comments and badge count after the PR was gone. Gate comments on `pr` existence so the list clears immediately.
|
83deb6d で対応しました。pr が null になったら comments も空配列にリセットするよう gate を追加しました。 |
-s ours merge to record that upstream commits a3e34bf through de70163 (13 commits) are semantically already present on origin/main via the PR1-6 cherry-pick series (PRs #176, #177, #178, #179, #180, #182), plus fork-adaptation fixes layered on top. This merge target is de70163 specifically (not upstream/main) so newer upstream commits (9fff075 and later) remain visible in future behind counts. Upstream commits covered by this audit merge: - a3e34bf fix(desktop): restore cmd+click requirement for v1 terminal file links (superset-sh#3457) [PR1/#176] - 57557f8 fix(desktop): gate v2 workspace children on collection readiness (superset-sh#3464) [PR1/#176] - 4ee2e61 fix(desktop): use native clipboard for copy path in v2 sidebar (superset-sh#3462) [PR1/#176] - 87d6e93 feat(desktop): close settings with Escape key (superset-sh#3466) [PR1/#176] - 9c7f5f4 chore(desktop): auto-restart host-service on bundle change in dev (superset-sh#3461) [PR1/#176] - 93140d9 fix(mcp): accept MCP resource URL as valid OAuth audience (superset-sh#3459) [PR2/#177] - be9e000 fix(desktop): drive tray menu off events, fetch real org name (superset-sh#3458) [PR2/#177] - c5f791e feat(v2): unify workspace delete through host-service (superset-sh#3443) [PR3/#178] - 2c24d93 feat(desktop): paginated branch picker with checkout + open actions (superset-sh#3397) [PR4/#179] - 2bf1049 feat(desktop/hotkeys): v1 directional pane focus + best-effort v1 override migrator (superset-sh#3460) [PR5/#180] - 1294a7d feat(desktop/hotkeys): restore Cmd+Alt+Arrow for tab/workspace nav (superset-sh#3472) [PR5/#180] - de70163 feat(desktop): v2 review tab first pass — PR info, checks, comments (superset-sh#3463) [PR6/#182] Intentionally skipped (version bump, fork has independent versioning): - 1e23353 chore(desktop): bump version to 1.5.5 (superset-sh#3473) Fork-adaptation fixes layered on top of the cherry-picks: - PR1: host-service-coordinator alias import fix, settings Escape selector narrowing (role-based + popper wrapper), Escape close uses replace navigation - PR2: dual quit mode preservation (requestQuit "release"/"stop"), trayUpdateToken guard for stale async fetchHostInfo results - PR4: ChangesHeader.normalizeBranchName regex rewrite (lint false positive), worktree add uses fullRef for remote-tracking refs, syncTimedOut reset on pendingId change, GIT_REFS.md barrel example fix - PR5: migrate.ts re-sanitize of existing localStorage overrides (v2 marker bump intent), FOCUS_PANE_* enabled:isActive for KeepAliveWorkspaces, CATEGORY_ORDER merges Navigation (upstream) and Browser (fork) - PR6: normalizeThreadsToComments flattens all thread.comments (not just first), CommentPane overrides <a> (openUrl) and <img> (SafeImage), zero-badge suppression, pr-null comments gate Fork features verified intact (Explore agent audit of combined 36d4de4..35d95f3 range): - BROWSER_RELOAD / BROWSER_HARD_RELOAD hotkeys - dual quit mode menu in tray - v1 terminal cold-restore + retry reconnect (out of range but unaffected) - KeepAliveWorkspaces (FOCUS_PANE_* gated on isActive) - useCommandPalette + addMemoTab in v2 workspace - host-service-coordinator rename alias pattern
概要
upstream superset-sh#3463
feat(desktop): v2 review tab first pass — PR info, checks, comments(commitde70163f5) を取り込みます。20ファイル +2031/-16。v2 workspace sidebar に v1 相当の Review タブを追加する単独機能追加コミット。主な変更
WorkspaceSidebar.tsx,SidebarHeader.tsx)hooks/useReviewTab/:useReviewTab.tsx,types.ts+ 4 component (PRHeader,ChecksSection,CommentsSection,ReviewTabContent)hooks/usePaneRegistry/components/CommentPane/:CommentPane.tsx,comment-pane.css(mermaid dark theme + code block stable identity)usePaneRegistry.tsxにcommentkind 追加v2-workspace/$workspaceId/page.tsxにopenCommentPanecallback 追加v2-workspace/$workspaceId/types.tsにCommentPaneData型追加apps/desktop/plans/20260413-1600-v2-review-tab.mdドキュメントv1 Review タブからの移植版。v2-native host-service endpoints (
git.getPullRequest,git.getPullRequestThreads) を使用。手動 conflict 解決
apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/page.tsxで content conflict。addMemoTabとuseCommandPalettehook ベースのhandleQuickOpenを持っていたopenCommentPane(新) とuseStateベースの簡易handleQuickOpenを入れたaddMemoTabとuseCommandPalette配線はそのまま、upstream のopenCommentPaneを追加 (FORK NOTE コメント付き)。openCommentPaneはWorkspaceSidebarのonOpenCommentprop に渡されるフォーク適応修正 (追加1コミット)
531e704b8 fix(fork): address Codex pre-review on PR6 review tabCodex 事前レビューで 4 件の問題を検出、3 件修正 + 1 件 defer しました。
1.
useReviewTab.tsxreview thread 返信コメント脱落 (High)upstream は
thread.comments[0]だけをNormalizedComment化しており、review thread 内の返信コメントが全て落ちていました。v1 は thread 内の全コメントを平坦化していたので同じ挙動に戻しました。2.
CommentPane.tsxXSS/navigation リスク (High)ReactMarkdownにcodeとtableしか差し替えていなかったため:<a>がデフォルトで main window を外部 URL に遷移させる可能性<img>が任意のリモート画像 (tracking pixel 含む) を読み込む可能性既存の共有
MarkdownRendererと同じガードを入れました:CommentLink:onClickでpreventDefault+electronTrpcClient.external.openUrlで外部ブラウザに逃がす (他の v2 Terminal pane 等と同じパターン)CommentImage:renderer/components/MarkdownRenderer/components/SafeImageでラップ。data:URL のみ許可3.
useReviewTab.tsxbadge が 0 を表示 (Minor)openCommentCountが 0 でもbadge: 0を返していたため、Review タブに0バッジが出ていました。Changes タブと同じくundefinedで抑制:Defer: review comment の GitHub URL 欠落
Codex が指摘した「review comments に GitHub URL が付かないので comment を GitHub で開く」問題は、
packages/host-service/src/trpc/router/git/の GraphQL query と types 両方の schema 変更が必要で、この cherry-pick のスコープ外 (host-service 側改修が必要)。upstream に別途フィードバックすべき問題。テスト
bun installbun run lint(biome + check-git-ref-strings pass、pre-existing projects.ts simple-git 違反のみ)bun run typecheck: /tmp worktree の @types/node 解決問題のため CI に委ねる<img>が data: URL 以外なら「Image blocked」表示 (fix fix(desktop): prevent browser webview reload on tab/workspace switch #2 検証)背景
PR1〜5 の 12 コミット取り込みが完了した後に upstream に追加された単独コミット。
Summary by CodeRabbit
リリースノート