feat(web): Commit diffs#1154
Merged
brendan-kellam merged 14 commits intomainfrom Apr 29, 2026
Merged
Conversation
This comment has been minimized.
This comment has been minimized.
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:
WalkthroughAdds a commit-diff viewer and related UI, extracts CodeMirror-based highlighting into a shared helper, introduces optional path filtering for /api/diff, gates react-grab script loading behind a new DEBUG_ENABLE_REACT_GRAP env var, and adjusts browse path handling to include a new commit path type. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Browser as Browse UI
participant Router
participant CommitPanel as FullCommitDiffPanel
participant GitAPI as Git API
participant Highlighter as CodeHighlighter
User->>Browser: Open commit URL
Browser->>Router: route to commit path
Router->>CommitPanel: render FullCommitDiffPanel(commitSha)
CommitPanel->>GitAPI: getCommit(commitSha)
GitAPI-->>CommitPanel: commit metadata
CommitPanel->>GitAPI: getDiff(base..head)
GitAPI-->>CommitPanel: diff response (files, hunks)
CommitPanel->>CommitPanel: compute change counts, authors
CommitPanel->>Highlighter: getCodeParserByFilename / highlightCode(hunks)
Highlighter-->>CommitPanel: highlighted segments
CommitPanel-->>Browser: render FileDiffList / LightweightDiffViewer
sequenceDiagram
participant User
participant Browse as Browse UI
participant CodePreview as CodePreviewPanel
participant GitAPI as Git API
participant Highlighter as CodeHighlighter
User->>Browse: Open blob with diff=true&previewRef=<sha>
Browse->>CodePreview: render with previewRef prop
CodePreview->>CodePreview: compute contentRef = previewRef ?? revisionName
CodePreview->>GitAPI: fetch file at contentRef
GitAPI-->>CodePreview: file content
CodePreview->>Highlighter: getCodeParserByFilename & highlightCode
Highlighter-->>CodePreview: styled spans
CodePreview-->>User: show preview banner + highlighted code + close preview link
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
Contributor
License Audit
Weak Copyleft Packages (informational)
Resolved Packages (10)
|
![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
🧹 Nitpick comments (8)
packages/web/src/app/layout.tsx (1)
47-59: Consider extracting the repeated debug predicate.Same condition is duplicated for both scripts; a shared local constant keeps this easier to maintain.
Refactor sketch
export default function RootLayout({ children, }: Readonly<{ children: React.ReactNode; }>) { + const isReactGrabDebugEnabled = + env.NODE_ENV === "development" && env.DEBUG_ENABLE_REACT_GRAP === "true"; + return ( <html @@ - {env.NODE_ENV === "development" && env.DEBUG_ENABLE_REACT_GRAP === 'true' && ( + {isReactGrabDebugEnabled && ( <Script src="//unpkg.com/react-grab/dist/index.global.js" crossOrigin="anonymous" strategy="beforeInteractive" /> )} - {env.NODE_ENV === "development" && env.DEBUG_ENABLE_REACT_GRAP === 'true' && ( + {isReactGrabDebugEnabled && ( <Script src="//unpkg.com/@react-grab/mcp/dist/client.global.js" strategy="lazyOnload" /> )}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/web/src/app/layout.tsx` around lines 47 - 59, Extract the duplicated predicate into a single local constant (e.g. isDevReactGrabEnabled) inside the component in layout.tsx and replace both occurrences of env.NODE_ENV === "development" && env.DEBUG_ENABLE_REACT_GRAP === 'true' with that constant; ensure the constant is computed before the JSX and used to gate both <Script ... /> renderings so the behavior is unchanged but maintenance is easier.packages/web/src/lib/codeHighlight.ts (1)
38-50: Cache parser lookup to avoid per-line resolution overhead.
highlightCoderesolves parser every call (Line 50), and it’s invoked per line inpackages/web/src/app/(app)/components/lightweightCodeHighlighter.tsx(Line 60-77). A small cache avoids repeated language matching/loading work.Proposed refactor
+const parserByLanguageNameCache = new Map<string, Promise<Parser>>(); + export const getCodeParserByLanguageName = async (languageName: string): Promise<Parser> => { - if (!languageName) { - return plainTextLanguage.parser; - } - const found = LanguageDescription.matchLanguageName(builtinLanguages, languageName, true); - if (!found) { - return plainTextLanguage.parser; - } - if (!found.support) { - try { - await found.load(); - } catch { - return plainTextLanguage.parser; - } - } - return found.support ? found.support.language.parser : plainTextLanguage.parser; + const key = languageName?.toLowerCase() ?? ''; + if (!parserByLanguageNameCache.has(key)) { + parserByLanguageNameCache.set( + key, + (async () => { + if (!languageName) { + return plainTextLanguage.parser; + } + const found = LanguageDescription.matchLanguageName(builtinLanguages, languageName, true); + if (!found) { + return plainTextLanguage.parser; + } + if (!found.support) { + try { + await found.load(); + } catch { + return plainTextLanguage.parser; + } + } + return found.support ? found.support.language.parser : plainTextLanguage.parser; + })(), + ); + } + return parserByLanguageNameCache.get(key)!; };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/web/src/lib/codeHighlight.ts` around lines 38 - 50, highlightCode currently calls getCodeParserByLanguageName on every invocation (causing heavy per-line overhead when invoked from lightweightCodeHighlighter), so introduce a simple module-level cache (e.g., a Map<string, Parser>) and use it inside highlightCode to return a cached parser if present; if missing, call getCodeParserByLanguageName, store the result in the cache, then proceed as before. Update references to highlightCode and ensure the cache key is the languageName string and the behavior unchanged for callers such as lightweightCodeHighlighter; no external API changes required.packages/web/src/app/(app)/components/pathHeader.tsx (1)
118-118: Consider replacing the hard-coded width reserve with a named constant.
40works, but this is easy to drift as button/icon spacing evolves. A named constant (or measured control width) would make future tweaks safer.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/web/src/app/`(app)/components/pathHeader.tsx at line 118, Replace the magic number 40 used when computing availableWidth (const availableWidth = containerWidth - 40) with a named constant (e.g., CONTROL_RESERVE_WIDTH or COPY_BUTTON_RESERVED_WIDTH) declared near the top of the PathHeader component or module; update the subtraction to use that constant and adjust the comment to explain what the reserved width covers (copy button, padding, icon spacing), or alternatively compute the reserve by measuring the control's offsetWidth if dynamic sizing is needed—this makes future spacing changes safer and easier to find.packages/web/src/app/(app)/browse/[...path]/components/commitHistoryPanel/commitRow.tsx (1)
62-67: Consider adding error handling for clipboard operations.The clipboard write could fail (e.g., due to permissions or secure context requirements). Currently, errors are silently ignored.
♻️ Suggested improvement
const onCopySha = useCallback(() => { - navigator.clipboard.writeText(commit.hash).then(() => { - toast({ description: "✅ Copied commit SHA to clipboard" }); - }); + navigator.clipboard.writeText(commit.hash) + .then(() => { + toast({ description: "✅ Copied commit SHA to clipboard" }); + }) + .catch(() => { + toast({ description: "❌ Failed to copy to clipboard", variant: "destructive" }); + }); return true; }, [commit.hash, toast]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/web/src/app/`(app)/browse/[...path]/components/commitHistoryPanel/commitRow.tsx around lines 62 - 67, The onCopySha callback currently calls navigator.clipboard.writeText(commit.hash) without handling failures; update onCopySha (the useCallback) to handle errors by awaiting or using .then/.catch on navigator.clipboard.writeText(commit.hash) and on failure show a toast error (e.g., toast({ variant: "destructive", description: "Failed to copy SHA" })) and optionally log the error; ensure you still return a boolean and keep dependencies [commit.hash, toast] unchanged.packages/web/src/app/(app)/browse/[...path]/page.tsx (1)
116-145: Consider extracting the panel selection into a helper or switch statement.The nested ternary chain for selecting which panel to render is functional but could become harder to maintain as more path types are added. A switch statement or mapping object would improve readability.
♻️ Optional refactor using a switch-like pattern
const renderPanel = () => { switch (browseProps.pathType) { case 'blob': return <CodePreviewPanel path={path} repoName={repoName} revisionName={revisionName} />; case 'commits': return <CommitsPanel path={path} repoName={repoName} revisionName={revisionName} page={page} author={author} since={since} until={until} />; case 'commit': return <CommitDiffPanel repoName={repoName} revisionName={revisionName} commitSha={browseProps.commitSha} path={path} />; case 'tree': return <TreePreviewPanel path={path} repoName={repoName} revisionName={revisionName} />; } };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/web/src/app/`(app)/browse/[...path]/page.tsx around lines 116 - 145, Extract the nested ternary into a small helper (e.g., renderPanel) that switches on browseProps.pathType and returns the appropriate panel component; replace the current inline ternary with a single call to renderPanel() and ensure the helper returns CodePreviewPanel, CommitsPanel (passing page, author, since, until), CommitDiffPanel (using browseProps.commitSha), or TreePreviewPanel as appropriate to keep prop passing identical.packages/web/src/app/(app)/browse/[...path]/components/commitDiffPanel/commitHashLine.tsx (1)
18-23: Same clipboard error handling suggestion applies here.This has the same pattern as
commitRow.tsx- consider adding error handling for the clipboard operation.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/web/src/app/`(app)/browse/[...path]/components/commitDiffPanel/commitHashLine.tsx around lines 18 - 23, The onCopyHash handler in commitHashLine.tsx calls navigator.clipboard.writeText(commitHash) without handling failures; update the onCopyHash function (the useCallback named onCopyHash) to handle rejected writes: either make it async and await writeText inside a try/catch or append a .catch handler, and on error call toast with an error description (and optionally log the error) while still returning an appropriate boolean; ensure you reference navigator.clipboard.writeText, commitHash, and toast in the new error path so users get feedback when the copy fails.packages/web/src/app/(app)/browse/[...path]/components/commitDiffPanel/commitDiffPanel.tsx (1)
27-27: UnusedrevisionNameprop.The
revisionNameparameter is declared in the props interface but never used in the component. Either remove it from the interface or use it if it's needed for future functionality.♻️ If unused, remove from props
-export const CommitDiffPanel = async ({ repoName, commitSha, path }: CommitDiffPanelProps) => { +export const CommitDiffPanel = async ({ repoName, revisionName, commitSha, path }: CommitDiffPanelProps) => {Or if truly not needed:
interface CommitDiffPanelProps { repoName: string; - revisionName?: string; commitSha: string; path: string; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/web/src/app/`(app)/browse/[...path]/components/commitDiffPanel/commitDiffPanel.tsx at line 27, The props interface includes an unused property revisionName referenced by CommitDiffPanelProps while CommitDiffPanel does not use it; remove revisionName from the CommitDiffPanelProps interface (or, if intended, add usage of revisionName inside the CommitDiffPanel component where repoName/commitSha/path are used) so the prop is either consumed or eliminated — update any call sites that pass revisionName accordingly and run type checks to ensure no remaining references.packages/web/src/app/(app)/browse/[...path]/components/commitDiffPanel/fileDiffList.tsx (1)
100-107: Redundantkeyprop onFileDiffRow.The
key={rowKey}onFileDiffRowis unnecessary since the parentdivalready haskey={virtualRow.key}. React only needs keys on the direct children of the mapped array.♻️ Remove redundant key
<FileDiffRow - key={rowKey} file={file} yOffset={virtualRow.start} repoName={repoName} commitSha={commitSha} parentSha={parentSha} />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/web/src/app/`(app)/browse/[...path]/components/commitDiffPanel/fileDiffList.tsx around lines 100 - 107, The FileDiffRow instance is receiving a redundant key prop (key={rowKey}) even though its parent element uses key={virtualRow.key}; remove the unnecessary key prop from the FileDiffRow JSX in fileDiffList.tsx so only the parent wrapper provides the key, leaving props file, yOffset, repoName, commitSha and parentSha unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/web/src/lib/codeHighlight.ts`:
- Around line 21-24: The code calls found.load() without handling rejections
which can break highlighting; update the logic in codeHighlight.ts so the await
found.load() is wrapped with error handling (try/catch or .catch) and if loading
fails or found.support is still falsy return plainTextLanguage.parser as a safe
fallback; apply the same change to the second occurrence around lines 32–35 (the
other branch that accesses found.support and found.load()) and reference
found.load(), found.support and plainTextLanguage.parser when making the change.
In `@packages/web/src/lib/utils.ts`:
- Around line 31-35: The regex currently truncates any string starting with 40
hex chars; change it to only treat a ref as a SHA when it is exactly 40 hex
chars or when it is followed by a non-alphanumeric separator. Replace the match
logic using ref.match(/^([0-9a-f]{40})(.*)$/i) with a regex that makes the
suffix optional and only captures it if it begins with a non-alphanumeric
character (for example /^([0-9a-f]{40})([^0-9A-Za-z].*)?$/i), then return
match[1].slice(0,7) + (match[2] || '') so symbolic refs that start with 40 hex
chars but have an alphanumeric suffix are not truncated.
---
Nitpick comments:
In
`@packages/web/src/app/`(app)/browse/[...path]/components/commitDiffPanel/commitDiffPanel.tsx:
- Line 27: The props interface includes an unused property revisionName
referenced by CommitDiffPanelProps while CommitDiffPanel does not use it; remove
revisionName from the CommitDiffPanelProps interface (or, if intended, add usage
of revisionName inside the CommitDiffPanel component where
repoName/commitSha/path are used) so the prop is either consumed or eliminated —
update any call sites that pass revisionName accordingly and run type checks to
ensure no remaining references.
In
`@packages/web/src/app/`(app)/browse/[...path]/components/commitDiffPanel/commitHashLine.tsx:
- Around line 18-23: The onCopyHash handler in commitHashLine.tsx calls
navigator.clipboard.writeText(commitHash) without handling failures; update the
onCopyHash function (the useCallback named onCopyHash) to handle rejected
writes: either make it async and await writeText inside a try/catch or append a
.catch handler, and on error call toast with an error description (and
optionally log the error) while still returning an appropriate boolean; ensure
you reference navigator.clipboard.writeText, commitHash, and toast in the new
error path so users get feedback when the copy fails.
In
`@packages/web/src/app/`(app)/browse/[...path]/components/commitDiffPanel/fileDiffList.tsx:
- Around line 100-107: The FileDiffRow instance is receiving a redundant key
prop (key={rowKey}) even though its parent element uses key={virtualRow.key};
remove the unnecessary key prop from the FileDiffRow JSX in fileDiffList.tsx so
only the parent wrapper provides the key, leaving props file, yOffset, repoName,
commitSha and parentSha unchanged.
In
`@packages/web/src/app/`(app)/browse/[...path]/components/commitHistoryPanel/commitRow.tsx:
- Around line 62-67: The onCopySha callback currently calls
navigator.clipboard.writeText(commit.hash) without handling failures; update
onCopySha (the useCallback) to handle errors by awaiting or using .then/.catch
on navigator.clipboard.writeText(commit.hash) and on failure show a toast error
(e.g., toast({ variant: "destructive", description: "Failed to copy SHA" })) and
optionally log the error; ensure you still return a boolean and keep
dependencies [commit.hash, toast] unchanged.
In `@packages/web/src/app/`(app)/browse/[...path]/page.tsx:
- Around line 116-145: Extract the nested ternary into a small helper (e.g.,
renderPanel) that switches on browseProps.pathType and returns the appropriate
panel component; replace the current inline ternary with a single call to
renderPanel() and ensure the helper returns CodePreviewPanel, CommitsPanel
(passing page, author, since, until), CommitDiffPanel (using
browseProps.commitSha), or TreePreviewPanel as appropriate to keep prop passing
identical.
In `@packages/web/src/app/`(app)/components/pathHeader.tsx:
- Line 118: Replace the magic number 40 used when computing availableWidth
(const availableWidth = containerWidth - 40) with a named constant (e.g.,
CONTROL_RESERVE_WIDTH or COPY_BUTTON_RESERVED_WIDTH) declared near the top of
the PathHeader component or module; update the subtraction to use that constant
and adjust the comment to explain what the reserved width covers (copy button,
padding, icon spacing), or alternatively compute the reserve by measuring the
control's offsetWidth if dynamic sizing is needed—this makes future spacing
changes safer and easier to find.
In `@packages/web/src/app/layout.tsx`:
- Around line 47-59: Extract the duplicated predicate into a single local
constant (e.g. isDevReactGrabEnabled) inside the component in layout.tsx and
replace both occurrences of env.NODE_ENV === "development" &&
env.DEBUG_ENABLE_REACT_GRAP === 'true' with that constant; ensure the constant
is computed before the JSX and used to gate both <Script ... /> renderings so
the behavior is unchanged but maintenance is easier.
In `@packages/web/src/lib/codeHighlight.ts`:
- Around line 38-50: highlightCode currently calls getCodeParserByLanguageName
on every invocation (causing heavy per-line overhead when invoked from
lightweightCodeHighlighter), so introduce a simple module-level cache (e.g., a
Map<string, Parser>) and use it inside highlightCode to return a cached parser
if present; if missing, call getCodeParserByLanguageName, store the result in
the cache, then proceed as before. Update references to highlightCode and ensure
the cache key is the languageName string and the behavior unchanged for callers
such as lightweightCodeHighlighter; no external API changes required.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 542c5684-421f-4dc6-8132-95d09095e896
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (36)
packages/shared/src/env.server.tspackages/web/package.jsonpackages/web/src/app/(app)/browse/[...path]/components/codePreviewPanel/codePreviewPanel.tsxpackages/web/src/app/(app)/browse/[...path]/components/codePreviewPanel/pureCodePreviewPanel.tsxpackages/web/src/app/(app)/browse/[...path]/components/codePreviewPanel/rangeHighlightingExtension.tspackages/web/src/app/(app)/browse/[...path]/components/commitDiffPanel/commitDiffPanel.tsxpackages/web/src/app/(app)/browse/[...path]/components/commitDiffPanel/commitHashLine.tsxpackages/web/src/app/(app)/browse/[...path]/components/commitDiffPanel/commitMessage.tsxpackages/web/src/app/(app)/browse/[...path]/components/commitDiffPanel/diffStat.tsxpackages/web/src/app/(app)/browse/[...path]/components/commitDiffPanel/fileDiffList.tsxpackages/web/src/app/(app)/browse/[...path]/components/commitDiffPanel/fileDiffRow.tsxpackages/web/src/app/(app)/browse/[...path]/components/commitDiffPanel/hunkParser.tspackages/web/src/app/(app)/browse/[...path]/components/commitDiffPanel/lightweightDiffViewer.tsxpackages/web/src/app/(app)/browse/[...path]/components/commitDiffPanel/splitPairing.tspackages/web/src/app/(app)/browse/[...path]/components/commitHistoryPanel/authorFilter.tsxpackages/web/src/app/(app)/browse/[...path]/components/commitHistoryPanel/commitRow.tsxpackages/web/src/app/(app)/browse/[...path]/components/commitHistoryPanel/commitsPagination.tsxpackages/web/src/app/(app)/browse/[...path]/components/commitHistoryPanel/commitsPanel.tsxpackages/web/src/app/(app)/browse/[...path]/components/commitHistoryPanel/dateFilter.tsxpackages/web/src/app/(app)/browse/[...path]/components/treePreviewPanel/pureTreePreviewPanel.tsxpackages/web/src/app/(app)/browse/[...path]/components/treePreviewPanel/treePreviewPanel.tsxpackages/web/src/app/(app)/browse/[...path]/page.tsxpackages/web/src/app/(app)/browse/components/historyRow.tsxpackages/web/src/app/(app)/browse/hooks/useBrowseNavigation.tspackages/web/src/app/(app)/browse/hooks/utils.tspackages/web/src/app/(app)/components/lightweightCodeHighlighter.tsxpackages/web/src/app/(app)/components/pathHeader.tsxpackages/web/src/app/layout.tsxpackages/web/src/features/chat/components/chatThread/tools/getDiffToolComponent.tsxpackages/web/src/features/git/getFilesApi.tspackages/web/src/features/git/getFolderContentsApi.tspackages/web/src/features/git/getTreeApi.tspackages/web/src/features/git/logger.tspackages/web/src/features/git/utils.tspackages/web/src/lib/codeHighlight.tspackages/web/src/lib/utils.ts
💤 Files with no reviewable changes (1)
- packages/web/src/features/git/utils.ts
Adds a `commit` pathType to the browse routes (`/browse/<repo>@<branch>/-/commit/<sha>[/<file>]`) that renders a placeholder CommitDiffPanel. Refactors browse path helpers into a discriminated `BrowseProps` union so commitSha is required only for pathType: 'commit'. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Wires up @codemirror/merge (via react-codemirror-merge) inside CommitDiffPanel with a static before/after demo. Adds a CodeDiff component that owns its language extension + view ref so each pane can reconfigure its language compartment independently. Also gates the react-grab dev scripts behind DEBUG_ENABLE_REACT_GRAP so they don't load on every dev page render. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Replace the CodeMirror MergeView-based commit diff rendering with a DOM based split-view that renders directly from git's hunks, inspired by Chrome DevTools' DiffView. Per-file editor instances and the matching getFileSource fetches are gone — a 50-file commit drops from ~100 network requests to 0, and per-row render cost from a full editor mount to a synchronous Lezer highlight + grid emit. - New `LightweightDiffViewer` builds a single 2-column subgrid with hunk headers spanning both sides; each cell uses `subgrid` so line numbers, markers, and content align across all rows. - Pure helpers split out: `hunkParser` (body string → DiffLine[]), `splitPairing` (DiffLine[] → SplitRow[]). - `presentableDiff` from @codemirror/merge supplies character-level intra-line diff highlighting on paired modifications. - Lezer highlight code lifted from `lightweightCodeHighlighter` into `lib/codeHighlight` so both files share the helper. - Drop `react-codemirror-merge` and `commitDiffEditor`. Long lines wrap via `whitespace-pre-wrap break-words` + `minmax(0, 1fr)` on the grid. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- File path header now sticks to the top of the scroll viewport while scrolling through that file's diff, using the negative-yOffset trick to compensate for the virtualizer's translateY positioning. Same pattern as searchResultsPanel/fileMatchContainer. - Lightweight diff viewer's grid uses `minmax(<min>, max-content)` for line-number and marker columns so they don't collapse to zero width when one side of the diff is entirely blank (fully-added or fully-deleted files), keeping the right pane aligned across files. - Drop the column gap between left and right panes and instead draw a `border-r` separator on the left cells for a cleaner divider. - Hunk header gets an optional className so the first hunk renders with just `border-b` (the file header above already provides the top border), while subsequent hunks render with `border-y` between them. - Drop the per-row footer padding in the virtualizer; rows now sit flush against each other. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- New `DiffStat` component renders GitHub-style additions/deletions counts with a 5-square indicator scaled log-ish to total change size. Added on the right of each file row header and on the right of the "N files changed" subheader for the commit total. Hidden when there are no line-level changes (pure renames). - Each file row gets a `CopyIconButton` next to the path (copies newPath, falling back to oldPath) and a `CommitActionLink` that opens the file at the commit. Deleted files link to the old path at the parent commit so the user lands on the file's last existing state rather than a 404. - `repoName`, `commitSha`, and `parentSha` are plumbed from the panel through `FileDiffList` to `FileDiffRow` to support the new link. - `computeChangeCounts` is memoized per file in the row. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…nchoring
History panel rows in both the bottom panel and the commits page are now
clickable — they navigate to the matching commit diff via router.push,
with closest('button, a') short-circuit so inner action buttons keep
their own behavior. Bottom-panel history rows also highlight via
bg-accent when their commit is the one currently being viewed.
Commit diff header now uses AuthorsAvatarGroup + getCommitAuthors +
formatAuthorsText, matching latestCommitInfo and historyRow — co-authors
parsed from the commit body show up correctly.
When the URL trailing path matches one of the commit's files, that file
is moved to the top of the FileDiffList rather than scrolled to. Avoids
estimateSize-based scroll inaccuracy and works regardless of which side
of a rename the URL points to.
Lightweight diff viewer short-circuits with "Diff too large to display"
for files containing lines over 1000 chars, matching the cutoff in
lightweightCodeHighlighter.
PathHeader's breadcrumb measurement reserved 175px for "copy button and
padding"; the actual reservation needed is ~40px. Reduced so breadcrumbs
no longer collapse prematurely on wide layouts.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Lift `truncateSha` (was a private helper in getDiffToolComponent) to `lib/utils` so PathHeader can reuse it. The branch/ref display now renders a 40-char SHA as `abc1234`, preserving any `^` / `~N` suffix. - Hide the `·` separator and the path's CopyIconButton when there's no path (repo root). Previously a dangling `·` and copy button rendered with nothing between them. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
brendan-kellam
force-pushed
the
bkellam/commit-diffs
branch
from
bb56039 to
5c36e73
Compare
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/web/src/app/(app)/components/lightweightCodeHighlighter.tsx (1)
138-139:⚠️ Potential issue | 🟡 MinorInvalid CSS value:
'none'is not valid forwhite-space.The
whiteSpaceproperty is set to'none', but'none'is not a valid CSS value forwhite-space. This will be ignored by browsers. Use'nowrap'to prevent text wrapping.Proposed fix
- whiteSpace: renderWhitespace ? 'pre-wrap' : 'none', + whiteSpace: renderWhitespace ? 'pre-wrap' : 'nowrap',🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/web/src/app/`(app)/components/lightweightCodeHighlighter.tsx around lines 138 - 139, The style object in the lightweightCodeHighlighter component sets whiteSpace to an invalid value ('none') when renderWhitespace is false; change that branch to a valid CSS value such as 'nowrap' so the line becomes whiteSpace: renderWhitespace ? 'pre-wrap' : 'nowrap' (locate the style usage in the component where whiteSpace and wordBreak are defined and update the renderWhitespace false-case).
🧹 Nitpick comments (5)
packages/shared/src/env.server.ts (1)
249-249: Fix typo in env key:DEBUG_ENABLE_REACT_GRAPshould beDEBUG_ENABLE_REACT_GRABThe env key misspells the package name (
react-grabis the correct spelling). The typo is actively used inpackages/web/src/app/layout.tsx(lines 47, 54). Introduce the corrected key with backward compatibility to avoid breaking existing deployments.Proposed backward-compatible change
- DEBUG_ENABLE_REACT_GRAP: booleanSchema.default('false'), + DEBUG_ENABLE_REACT_GRAB: booleanSchema + .optional() + .transform(value => { + return value ?? ((process.env.DEBUG_ENABLE_REACT_GRAP as 'true' | 'false') ?? 'false'); + }), + /** + * `@deprecated` Use `DEBUG_ENABLE_REACT_GRAB` instead. + */ + DEBUG_ENABLE_REACT_GRAP: booleanSchema.optional(),After migration, update
packages/web/src/app/layout.tsxto reference the corrected key.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/shared/src/env.server.ts` at line 249, The env schema has a typo: DEBUG_ENABLE_REACT_GRAP should be DEBUG_ENABLE_REACT_GRAB; update the schema in env.server.ts to introduce DEBUG_ENABLE_REACT_GRAB:booleanSchema.default('false') while keeping the old DEBUG_ENABLE_REACT_GRAP:booleanSchema.optional() (or mapping its value into the new key) so existing deployments remain working; ensure the code that reads the flag (references in packages/web/src/app/layout.tsx) is updated to use DEBUG_ENABLE_REACT_GRAB after you add the backward-compatibility mapping so runtime behavior is unchanged for environments still setting the old variable.packages/web/src/app/(app)/browse/[...path]/components/commitDiffPanel/commitDiffPanel.tsx (1)
16-27: Remove unusedrevisionNameprop or use it.The
revisionNameprop is destructured in the interface but never used in the component body. Either remove it from the interface or use it where appropriate (e.g., in the "Browse files" link if different fromcommitResponse.hash).If unused, remove from interface
interface CommitDiffPanelProps { repoName: string; - revisionName?: string; commitSha: string; path: string; } -export const CommitDiffPanel = async ({ repoName, commitSha, path }: CommitDiffPanelProps) => { +export const CommitDiffPanel = async ({ repoName, commitSha, path }: CommitDiffPanelProps) => {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/web/src/app/`(app)/browse/[...path]/components/commitDiffPanel/commitDiffPanel.tsx around lines 16 - 27, The CommitDiffPanelProps interface declares revisionName but the CommitDiffPanel component never uses it; either remove revisionName from CommitDiffPanelProps to eliminate the unused prop, or thread revisionName into the component (e.g., use the revisionName value instead of or as a fallback to commitResponse.hash when building the "Browse files" link inside CommitDiffPanel) so the prop is actually consumed; update usages/call sites accordingly and remove any linter warnings.packages/web/src/app/(app)/browse/[...path]/components/commitDiffPanel/fileDiffList.tsx (1)
14-15: TODO: Scroll-to-target functionality is documented but not implemented.The interface comment says "When set, scroll the matching file into view on mount" but no scrolling logic exists. Either implement the scroll behavior or update the comment to reflect current behavior.
Would you like me to help implement the scroll-to-target functionality using
virtualizer.scrollToIndex()?🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/web/src/app/`(app)/browse/[...path]/components/commitDiffPanel/fileDiffList.tsx around lines 14 - 15, The comment on the targetPath prop promises scroll-to-target-on-mount but there's no implementation; implement scrolling in the FileDiffList component by detecting targetPath prop on mount/update and calling the virtualizer.scrollToIndex() for the matching item index (use the same key/index logic as used in renderRow or getItemIndex) and ensure this runs after items are rendered (e.g., inside useEffect tied to targetPath and item list length); alternatively, if you choose not to implement, update the comment to remove the mount scrolling claim and document current behavior instead.packages/web/src/app/(app)/browse/[...path]/components/commitDiffPanel/fileDiffRow.tsx (1)
74-77: Clarify the negativetopoffset for sticky positioning.The
top: -${yOffset}pxcreates a negative offset relative to the scroll container. This appears to compensate for the virtual row's absolute positioning, but could be confusing. Consider adding a comment explaining why this negative offset is needed for the sticky header to work correctly within the virtualized list.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/web/src/app/`(app)/browse/[...path]/components/commitDiffPanel/fileDiffRow.tsx around lines 74 - 77, The sticky header div in fileDiffRow.tsx uses a negative top offset (style: top: `-${yOffset}px`) to counteract the virtual row's absolute positioning so the header remains visible in the virtualized list; add an inline comment immediately above or next to the div (the element with className "flex flex-row items-center... sticky z-10") explaining that the negative top compensates for the virtual row's offset/absolute positioning and is required for correct sticky behavior inside the virtual scroller (mention yOffset variable), so future readers understand why a negative value is used.packages/web/src/app/(app)/browse/[...path]/components/commitDiffPanel/lightweightDiffViewer.tsx (1)
235-290: Consider extracting shared highlighting logic.The
renderLineContentfunction duplicates the pattern fromhighlightCodeincodeHighlight.ts(tree parsing,highlightTreewalk, gap filling, range splitting). While the overlay semantics differ (innerDiffClassvssearchMatch-selected), the core token-splitting logic could be generalized.This is fine for now given the different requirements, but worth considering if more callers emerge.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/web/src/app/`(app)/browse/[...path]/components/commitDiffPanel/lightweightDiffViewer.tsx around lines 235 - 290, renderLineContent duplicates tokenization/highlighting patterns from highlightCode (tree parsing, highlightTree walk, gap filling, and range splitting) — extract the shared logic into a reusable helper (e.g., a function in codeHighlight.ts) that accepts the parsed tree/content, a highlighter, gap-emission behavior, and an overlay-range mapper; update renderLineContent to call that helper with its specific overlay semantics (innerDiffRanges and innerDiffClass) and update highlightCode to reuse the same helper for searchMatch-selected behavior, keeping unique overlay application (innerDiffClass vs searchMatch-selected) as pluggable strategy callbacks.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@packages/web/src/app/`(app)/components/lightweightCodeHighlighter.tsx:
- Around line 138-139: The style object in the lightweightCodeHighlighter
component sets whiteSpace to an invalid value ('none') when renderWhitespace is
false; change that branch to a valid CSS value such as 'nowrap' so the line
becomes whiteSpace: renderWhitespace ? 'pre-wrap' : 'nowrap' (locate the style
usage in the component where whiteSpace and wordBreak are defined and update the
renderWhitespace false-case).
---
Nitpick comments:
In `@packages/shared/src/env.server.ts`:
- Line 249: The env schema has a typo: DEBUG_ENABLE_REACT_GRAP should be
DEBUG_ENABLE_REACT_GRAB; update the schema in env.server.ts to introduce
DEBUG_ENABLE_REACT_GRAB:booleanSchema.default('false') while keeping the old
DEBUG_ENABLE_REACT_GRAP:booleanSchema.optional() (or mapping its value into the
new key) so existing deployments remain working; ensure the code that reads the
flag (references in packages/web/src/app/layout.tsx) is updated to use
DEBUG_ENABLE_REACT_GRAB after you add the backward-compatibility mapping so
runtime behavior is unchanged for environments still setting the old variable.
In
`@packages/web/src/app/`(app)/browse/[...path]/components/commitDiffPanel/commitDiffPanel.tsx:
- Around line 16-27: The CommitDiffPanelProps interface declares revisionName
but the CommitDiffPanel component never uses it; either remove revisionName from
CommitDiffPanelProps to eliminate the unused prop, or thread revisionName into
the component (e.g., use the revisionName value instead of or as a fallback to
commitResponse.hash when building the "Browse files" link inside
CommitDiffPanel) so the prop is actually consumed; update usages/call sites
accordingly and remove any linter warnings.
In
`@packages/web/src/app/`(app)/browse/[...path]/components/commitDiffPanel/fileDiffList.tsx:
- Around line 14-15: The comment on the targetPath prop promises
scroll-to-target-on-mount but there's no implementation; implement scrolling in
the FileDiffList component by detecting targetPath prop on mount/update and
calling the virtualizer.scrollToIndex() for the matching item index (use the
same key/index logic as used in renderRow or getItemIndex) and ensure this runs
after items are rendered (e.g., inside useEffect tied to targetPath and item
list length); alternatively, if you choose not to implement, update the comment
to remove the mount scrolling claim and document current behavior instead.
In
`@packages/web/src/app/`(app)/browse/[...path]/components/commitDiffPanel/fileDiffRow.tsx:
- Around line 74-77: The sticky header div in fileDiffRow.tsx uses a negative
top offset (style: top: `-${yOffset}px`) to counteract the virtual row's
absolute positioning so the header remains visible in the virtualized list; add
an inline comment immediately above or next to the div (the element with
className "flex flex-row items-center... sticky z-10") explaining that the
negative top compensates for the virtual row's offset/absolute positioning and
is required for correct sticky behavior inside the virtual scroller (mention
yOffset variable), so future readers understand why a negative value is used.
In
`@packages/web/src/app/`(app)/browse/[...path]/components/commitDiffPanel/lightweightDiffViewer.tsx:
- Around line 235-290: renderLineContent duplicates tokenization/highlighting
patterns from highlightCode (tree parsing, highlightTree walk, gap filling, and
range splitting) — extract the shared logic into a reusable helper (e.g., a
function in codeHighlight.ts) that accepts the parsed tree/content, a
highlighter, gap-emission behavior, and an overlay-range mapper; update
renderLineContent to call that helper with its specific overlay semantics
(innerDiffRanges and innerDiffClass) and update highlightCode to reuse the same
helper for searchMatch-selected behavior, keeping unique overlay application
(innerDiffClass vs searchMatch-selected) as pluggable strategy callbacks.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d312fa33-3440-4943-91e0-3eec894115e8
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (36)
packages/shared/src/env.server.tspackages/web/package.jsonpackages/web/src/app/(app)/browse/[...path]/components/codePreviewPanel/codePreviewPanel.tsxpackages/web/src/app/(app)/browse/[...path]/components/codePreviewPanel/pureCodePreviewPanel.tsxpackages/web/src/app/(app)/browse/[...path]/components/codePreviewPanel/rangeHighlightingExtension.tspackages/web/src/app/(app)/browse/[...path]/components/commitDiffPanel/commitDiffPanel.tsxpackages/web/src/app/(app)/browse/[...path]/components/commitDiffPanel/commitHashLine.tsxpackages/web/src/app/(app)/browse/[...path]/components/commitDiffPanel/commitMessage.tsxpackages/web/src/app/(app)/browse/[...path]/components/commitDiffPanel/diffStat.tsxpackages/web/src/app/(app)/browse/[...path]/components/commitDiffPanel/fileDiffList.tsxpackages/web/src/app/(app)/browse/[...path]/components/commitDiffPanel/fileDiffRow.tsxpackages/web/src/app/(app)/browse/[...path]/components/commitDiffPanel/hunkParser.tspackages/web/src/app/(app)/browse/[...path]/components/commitDiffPanel/lightweightDiffViewer.tsxpackages/web/src/app/(app)/browse/[...path]/components/commitDiffPanel/splitPairing.tspackages/web/src/app/(app)/browse/[...path]/components/commitHistoryPanel/authorFilter.tsxpackages/web/src/app/(app)/browse/[...path]/components/commitHistoryPanel/commitRow.tsxpackages/web/src/app/(app)/browse/[...path]/components/commitHistoryPanel/commitsPagination.tsxpackages/web/src/app/(app)/browse/[...path]/components/commitHistoryPanel/commitsPanel.tsxpackages/web/src/app/(app)/browse/[...path]/components/commitHistoryPanel/dateFilter.tsxpackages/web/src/app/(app)/browse/[...path]/components/treePreviewPanel/pureTreePreviewPanel.tsxpackages/web/src/app/(app)/browse/[...path]/components/treePreviewPanel/treePreviewPanel.tsxpackages/web/src/app/(app)/browse/[...path]/page.tsxpackages/web/src/app/(app)/browse/components/historyRow.tsxpackages/web/src/app/(app)/browse/hooks/useBrowseNavigation.tspackages/web/src/app/(app)/browse/hooks/utils.tspackages/web/src/app/(app)/components/lightweightCodeHighlighter.tsxpackages/web/src/app/(app)/components/pathHeader.tsxpackages/web/src/app/layout.tsxpackages/web/src/features/chat/components/chatThread/tools/getDiffToolComponent.tsxpackages/web/src/features/git/getFilesApi.tspackages/web/src/features/git/getFolderContentsApi.tspackages/web/src/features/git/getTreeApi.tspackages/web/src/features/git/logger.tspackages/web/src/features/git/utils.tspackages/web/src/lib/codeHighlight.tspackages/web/src/lib/utils.ts
💤 Files with no reviewable changes (1)
- packages/web/src/features/git/utils.ts
✅ Files skipped from review due to trivial changes (12)
- packages/web/src/features/git/getTreeApi.ts
- packages/web/src/app/(app)/browse/[...path]/components/codePreviewPanel/rangeHighlightingExtension.ts
- packages/web/src/features/git/getFilesApi.ts
- packages/web/src/app/(app)/browse/[...path]/components/treePreviewPanel/pureTreePreviewPanel.tsx
- packages/web/src/features/chat/components/chatThread/tools/getDiffToolComponent.tsx
- packages/web/src/app/(app)/browse/[...path]/components/codePreviewPanel/pureCodePreviewPanel.tsx
- packages/web/src/app/(app)/browse/[...path]/components/commitHistoryPanel/commitsPanel.tsx
- packages/web/src/features/git/getFolderContentsApi.ts
- packages/web/src/features/git/logger.ts
- packages/web/package.json
- packages/web/src/app/(app)/browse/[...path]/components/commitDiffPanel/splitPairing.ts
- packages/web/src/app/(app)/browse/[...path]/components/commitDiffPanel/hunkParser.ts
🚧 Files skipped from review as they are similar to previous changes (9)
- packages/web/src/lib/utils.ts
- packages/web/src/app/(app)/browse/[...path]/components/commitDiffPanel/commitMessage.tsx
- packages/web/src/app/(app)/browse/[...path]/components/commitDiffPanel/commitHashLine.tsx
- packages/web/src/app/(app)/browse/hooks/useBrowseNavigation.ts
- packages/web/src/app/(app)/browse/[...path]/components/commitHistoryPanel/commitRow.tsx
- packages/web/src/app/(app)/browse/components/historyRow.tsx
- packages/web/src/app/layout.tsx
- packages/web/src/app/(app)/browse/[...path]/page.tsx
- packages/web/src/app/(app)/components/pathHeader.tsx
Adds a `path` query/tool parameter to restrict diff output to changes touching a single file via git's `-- <pathspec>` separator. Refactors the route handler to use the shared `getDiffRequestSchema`. Fixes SOU-1154 (#1154) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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 `@packages/web/src/features/git/getDiffApi.ts`:
- Around line 68-74: The current truthy check “if (path)” can treat an empty
string as no filter and run a full diff; change the condition guarding
diffArgs.push('--', path) to explicitly check for undefined (e.g., path !==
undefined) so that empty-string inputs are preserved and do not silently widen
the diff scope; update the block around the diffArgs array construction (where
diffArgs is defined and where push('--', path) is called) to use the explicit
undefined check.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 8d9b9916-1374-4f7f-a4e4-540883bab240
📒 Files selected for processing (6)
CHANGELOG.mddocs/api-reference/sourcebot-public.openapi.jsonpackages/web/src/app/api/(server)/diff/route.tspackages/web/src/features/git/getDiffApi.tspackages/web/src/features/git/schemas.tspackages/web/src/features/tools/getDiff.ts
✅ Files skipped from review due to trivial changes (3)
- CHANGELOG.md
- packages/web/src/features/git/schemas.ts
- docs/api-reference/sourcebot-public.openapi.json
- Move single-file commit diffs from `/-/commit/<sha>/<path>` to
`/-/blob/<path>?ref=<sha>&diff=true`, keeping the user's browsing
revision in the URL. `/-/commit/<sha>` still renders the full
multi-file diff.
- New `FocusedCommitDiffPanel` with status row (file status badge +
authors + relative commit date + "View full commit" + DiffStat +
exit-X) and path-filtered `getDiff` so only the single file's diff
is fetched.
- New preview banner in `CodePreviewPanel` when `?ref=` is set, with a
close button that strips the param.
- Make `PathHeader`'s revision clickable, linking to that ref's full
commit view.
- New `HoverPrefetchLink` defers Next.js prefetching until hover; used
in history rows to avoid firing many prefetches on render.
- Hide the bottom panel on `/-/commit/` views.
- Extract `getFileStatus` / `StatusBadge` to a shared `fileStatus.tsx`.
- Workaround Radix Tooltip + RSC re-render bug (drop `asChild` from
`<TooltipTrigger>`, add `key={commitSha}`) so X / Browse-files
buttons survive client navigation between commits.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (3)
packages/web/src/app/(app)/components/hoverPrefetchLink.tsx (2)
12-24: Consider enabling activation on keyboard focus, not just hover.Line 12 and Lines 21-24 only flip
activeononMouseEnter. Keyboard users tabbing through links won’t trigger this path. AddingonFocuskeeps behavior consistent across input methods.Optional accessibility tweak
export const HoverPrefetchLink = ({ onMouseEnter, + onFocus, ...props }: HoverPrefetchLinkProps) => { @@ onMouseEnter={(event) => { setActive(true); onMouseEnter?.(event); }} + onFocus={(event) => { + setActive(true); + onFocus?.(event); + }} />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/web/src/app/`(app)/components/hoverPrefetchLink.tsx around lines 12 - 24, The component HoverPrefetchLink only sets active via onMouseEnter, so keyboard users won't trigger prefetch; add an onFocus handler that also calls setActive(true) and invokes any provided onFocus prop, and add symmetric onMouseLeave and onBlur handlers to call setActive(false) (invoking onMouseLeave/onBlur props if present) so prefetched state works for both mouse and keyboard navigation while preserving existing prop callbacks.
6-27:HoverPrefetchLinkis not fully drop-in becauserefcannot be passed.Line 6 + Lines 11-27 remove
refsupport (ComponentPropsWithoutRefand noforwardRef). If this is intended to be a drop-innext/linkreplacement, forward the anchor ref.Proposed ref-forwarding update
-import { ComponentPropsWithoutRef, useState } from "react"; +import { ComponentPropsWithRef, forwardRef, useState } from "react"; -type HoverPrefetchLinkProps = Omit<ComponentPropsWithoutRef<typeof Link>, 'prefetch'>; +type HoverPrefetchLinkProps = Omit<ComponentPropsWithRef<typeof Link>, "prefetch">; -export const HoverPrefetchLink = ({ - onMouseEnter, - ...props -}: HoverPrefetchLinkProps) => { +export const HoverPrefetchLink = forwardRef<HTMLAnchorElement, HoverPrefetchLinkProps>(function HoverPrefetchLink({ + onMouseEnter, + ...props +}, ref) { const [active, setActive] = useState(false); return ( <Link + ref={ref} {...props} prefetch={active ? true : false} onMouseEnter={(event) => { setActive(true); onMouseEnter?.(event); }} /> ); -}; +});🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/web/src/app/`(app)/components/hoverPrefetchLink.tsx around lines 6 - 27, HoverPrefetchLink currently drops support for refs (uses ComponentPropsWithoutRef and a plain function) so it isn't a true drop-in for next/link; convert it to a forwardRef component and accept/passthrough the anchor ref. Change the props type to Omit<ComponentPropsWithRef<typeof Link>,'prefetch'>, wrap the component with forwardRef<HTMLAnchorElement, HoverPrefetchLinkProps>(function HoverPrefetchLink(props, ref) { ... }), accept the ref param and pass it into <Link ref={ref} ... />, and preserve the onMouseEnter handling and prefetch logic so ref consumers (e.g., parent DOM access or focus management) continue to work.packages/web/src/app/(app)/browse/[...path]/components/commitDiffPanel/fullCommitDiffPanel.tsx (1)
21-24: ExtractEMPTY_TREE_SHAinto a shared constant.This value is duplicated in both commit diff panel files. Centralizing it avoids drift and keeps fallback behavior consistent.
Refactor sketch
-const EMPTY_TREE_SHA = '4b825dc642cb6eb9a060e54bf8d69288fbee4904'; +import { EMPTY_TREE_SHA } from "./gitConstants";+// packages/web/src/app/(app)/browse/[...path]/components/commitDiffPanel/gitConstants.ts +export const EMPTY_TREE_SHA = '4b825dc642cb6eb9a060e54bf8d69288fbee4904';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/web/src/app/`(app)/browse/[...path]/components/commitDiffPanel/fullCommitDiffPanel.tsx around lines 21 - 24, Extract the duplicated EMPTY_TREE_SHA string into a single exported constant and import it where needed: create a shared constant (e.g., export const EMPTY_TREE_SHA = '4b825dc642cb6eb9a060e54bf8d69288fbee4904') in a central module (shared constants file or utils) then replace the local declaration in fullCommitDiffPanel.tsx (symbol EMPTY_TREE_SHA) and the other commit diff panel file with an import from that module; ensure the new export is named exactly EMPTY_TREE_SHA so existing usages require only an import change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@packages/web/src/app/`(app)/browse/[...path]/components/codePreviewPanel/codePreviewPanel.tsx:
- Around line 104-123: The TooltipTrigger currently wraps an interactive Button
(which contains a Link), producing nested buttons; update the TooltipTrigger
usage in codePreviewPanel.tsx by adding the asChild prop to the TooltipTrigger
component so it will render its child Button directly (matching other usages),
ensuring TooltipTrigger, Button and Link (the elements around getBrowsePath(...)
and X icon) are not nested interactive elements and restore valid HTML/keyboard
accessibility.
In
`@packages/web/src/app/`(app)/browse/[...path]/components/commitDiffPanel/focusedCommitDiffPanel.tsx:
- Around line 155-176: The TooltipTrigger surrounding the Button should include
the asChild prop to follow the established composition pattern and avoid an
extra wrapper element; update the TooltipTrigger (the one keyed by commitSha
that wraps Button asChild which contains the Link returned from getBrowsePath)
to be <TooltipTrigger asChild> so the Button is rendered as the trigger element
directly.
In
`@packages/web/src/app/`(app)/browse/[...path]/components/commitDiffPanel/fullCommitDiffPanel.tsx:
- Around line 78-94: The TooltipTrigger currently wraps a Button (which renders
a Link) causing nested interactive elements; update TooltipTrigger to include
the asChild prop so it forwards the trigger to the Button/Link element (e.g.,
change TooltipTrigger to <TooltipTrigger asChild> around the Button in
fullCommitDiffPanel.tsx and do the same in focusedCommitDiffPanel.tsx at the
referenced locations) to prevent invalid <button><a> nesting and ensure correct
accessibility/HTML structure.
In
`@packages/web/src/app/`(app)/browse/[...path]/components/commitHistoryPanel/commitRow.tsx:
- Around line 61-77: The commit row currently uses a clickable div (onRowClick)
which is not keyboard accessible; update the row container element used in
commitRow.tsx to include role="link" and tabIndex={0}, and add an onKeyDown
handler that mirrors onRowClick: ignore key events coming from interactive
children (use event.target.closest('button, a')), handle Enter by calling
router.push(commitDiffHref), and handle Space by preventDefault() then call
router.push(commitDiffHref); reuse the same navigation logic from onRowClick (or
extract a navigateToCommit function) so both onClick and onKeyDown trigger
identical navigation behavior.
---
Nitpick comments:
In
`@packages/web/src/app/`(app)/browse/[...path]/components/commitDiffPanel/fullCommitDiffPanel.tsx:
- Around line 21-24: Extract the duplicated EMPTY_TREE_SHA string into a single
exported constant and import it where needed: create a shared constant (e.g.,
export const EMPTY_TREE_SHA = '4b825dc642cb6eb9a060e54bf8d69288fbee4904') in a
central module (shared constants file or utils) then replace the local
declaration in fullCommitDiffPanel.tsx (symbol EMPTY_TREE_SHA) and the other
commit diff panel file with an import from that module; ensure the new export is
named exactly EMPTY_TREE_SHA so existing usages require only an import change.
In `@packages/web/src/app/`(app)/components/hoverPrefetchLink.tsx:
- Around line 12-24: The component HoverPrefetchLink only sets active via
onMouseEnter, so keyboard users won't trigger prefetch; add an onFocus handler
that also calls setActive(true) and invokes any provided onFocus prop, and add
symmetric onMouseLeave and onBlur handlers to call setActive(false) (invoking
onMouseLeave/onBlur props if present) so prefetched state works for both mouse
and keyboard navigation while preserving existing prop callbacks.
- Around line 6-27: HoverPrefetchLink currently drops support for refs (uses
ComponentPropsWithoutRef and a plain function) so it isn't a true drop-in for
next/link; convert it to a forwardRef component and accept/passthrough the
anchor ref. Change the props type to Omit<ComponentPropsWithRef<typeof
Link>,'prefetch'>, wrap the component with forwardRef<HTMLAnchorElement,
HoverPrefetchLinkProps>(function HoverPrefetchLink(props, ref) { ... }), accept
the ref param and pass it into <Link ref={ref} ... />, and preserve the
onMouseEnter handling and prefetch logic so ref consumers (e.g., parent DOM
access or focus management) continue to work.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 99667ea6-2b1f-48e4-887b-bf085727b375
📒 Files selected for processing (17)
CHANGELOG.mdpackages/web/src/app/(app)/browse/[...path]/components/codePreviewPanel/codePreviewPanel.tsxpackages/web/src/app/(app)/browse/[...path]/components/commitDiffPanel/fileDiffList.tsxpackages/web/src/app/(app)/browse/[...path]/components/commitDiffPanel/fileDiffRow.tsxpackages/web/src/app/(app)/browse/[...path]/components/commitDiffPanel/fileStatus.tsxpackages/web/src/app/(app)/browse/[...path]/components/commitDiffPanel/focusedCommitDiffPanel.tsxpackages/web/src/app/(app)/browse/[...path]/components/commitDiffPanel/fullCommitDiffPanel.tsxpackages/web/src/app/(app)/browse/[...path]/components/commitHistoryPanel/commitRow.tsxpackages/web/src/app/(app)/browse/[...path]/components/commitHistoryPanel/commitsPanel.tsxpackages/web/src/app/(app)/browse/[...path]/page.tsxpackages/web/src/app/(app)/browse/components/bottomPanel.tsxpackages/web/src/app/(app)/browse/components/historyPanel.tsxpackages/web/src/app/(app)/browse/components/historyRow.tsxpackages/web/src/app/(app)/browse/hooks/utils.tspackages/web/src/app/(app)/browse/layoutClient.tsxpackages/web/src/app/(app)/components/hoverPrefetchLink.tsxpackages/web/src/app/(app)/components/pathHeader.tsx
💤 Files with no reviewable changes (1)
- packages/web/src/app/(app)/browse/components/bottomPanel.tsx
✅ Files skipped from review due to trivial changes (1)
- CHANGELOG.md
🚧 Files skipped from review as they are similar to previous changes (5)
- packages/web/src/app/(app)/browse/[...path]/components/commitHistoryPanel/commitsPanel.tsx
- packages/web/src/app/(app)/components/pathHeader.tsx
- packages/web/src/app/(app)/browse/components/historyRow.tsx
- packages/web/src/app/(app)/browse/[...path]/components/commitDiffPanel/fileDiffList.tsx
- packages/web/src/app/(app)/browse/[...path]/page.tsx
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
packages/web/src/app/(app)/browse/[...path]/components/commitHistoryPanel/commitRow.tsx (1)
68-77:⚠️ Potential issue | 🟡 MinorGuard
event.targetbefore callingclosest()to avoid runtime crashes.At Line 69 and Line 77,
event.target as HTMLElementis an unsafe cast. If the target is not anElement,.closest()can throw and break navigation handlers.Proposed fix
const onRowClick = useCallback((event: React.MouseEvent<HTMLDivElement>) => { - const target = event.target as HTMLElement; - if (target.closest('button, a')) { + if (!(event.target instanceof Element) || event.target.closest('button, a')) { return; } navigateToCommit(); }, [navigateToCommit]); const onRowKeyDown = useCallback((event: React.KeyboardEvent<HTMLDivElement>) => { - if ((event.target as HTMLElement).closest('button, a')) { + if (!(event.target instanceof Element) || event.target.closest('button, a')) { return; } if (event.key === 'Enter') { navigateToCommit(); } }, [navigateToCommit]);#!/bin/bash # Verify the unsafe cast locations in CommitRow handlers (read-only). FILE='packages/web/src/app/(app)/browse/[...path]/components/commitHistoryPanel/commitRow.tsx' nl -ba "$FILE" | sed -n '66,84p' rg -n --type=tsx 'event\.target as HTMLElement' "$FILE"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/web/src/app/`(app)/browse/[...path]/components/commitHistoryPanel/commitRow.tsx around lines 68 - 77, The handlers onRowClick and onRowKeyDown cast event.target unsafely before calling .closest(); update both to first guard that event.target is an Element (e.g., check "if (!(event.target instanceof Element)) return;" or "const target = event.target; if (!target || !(target as Element).closest) return;") then call target.closest('button, a') and proceed to navigateToCommit; ensure you reference the same variables (onRowClick, onRowKeyDown, navigateToCommit) and keep types compatible with React.MouseEvent/React.KeyboardEvent while avoiding unsafe HTMLElement casts.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In
`@packages/web/src/app/`(app)/browse/[...path]/components/commitHistoryPanel/commitRow.tsx:
- Around line 68-77: The handlers onRowClick and onRowKeyDown cast event.target
unsafely before calling .closest(); update both to first guard that event.target
is an Element (e.g., check "if (!(event.target instanceof Element)) return;" or
"const target = event.target; if (!target || !(target as Element).closest)
return;") then call target.closest('button, a') and proceed to navigateToCommit;
ensure you reference the same variables (onRowClick, onRowKeyDown,
navigateToCommit) and keep types compatible with
React.MouseEvent/React.KeyboardEvent while avoiding unsafe HTMLElement casts.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 85d2f282-2697-4038-ad10-b4eed273896f
📒 Files selected for processing (1)
packages/web/src/app/(app)/browse/[...path]/components/commitHistoryPanel/commitRow.tsx
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
1 participant
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.
Focused commit view (diff):
Focused commit view (file @ rev):
Full diff view:
Summary by CodeRabbit
Release Notes
New Features
Enhancements
Documentation