[design-doctor effort xhigh] replacement clone of sourcebot#2: [design-doctor test] sourcebot-1154: Commit diff viewer#30
Conversation
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>
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 (sourcebot-dev#1154) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- 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>
| <CommitMessage subject={subject} body={commitResponse.body} /> | ||
| </div> | ||
| <Tooltip key={commitSha}> | ||
| <TooltipTrigger> |
There was a problem hiding this comment.
Commit diff buttons sit inside an extra clickable wrapper
The close, exit-diff, and browse buttons in the commit diff view are each wrapped in an extra interactive layer instead of being a single self-contained control. This can add a redundant focus stop and make the buttons feel slightly off compared with the other icon and action buttons in the file browser. After the fix they should behave like one clean, consistent control.
Prompt to fix with AI
This is a comment left during a design review.
Comment:
**Commit diff buttons sit inside an extra clickable wrapper**
The close, exit-diff, and browse buttons in the commit diff view are each wrapped in an extra interactive layer instead of being a single self-contained control. This can add a redundant focus stop and make the buttons feel slightly off compared with the other icon and action buttons in the file browser. After the fix they should behave like one clean, consistent control.
Issue:
All three new Tooltip-wrapped buttons render `<Tooltip><TooltipTrigger><Button asChild ...><Link>...`. Because TooltipTrigger (Radix Trigger) has no `asChild`, it renders its own <button> that wraps the Button's <Link> <a>, producing a nested button>anchor and an extra interactive wrapper around the control. Sites: fullCommitDiffPanel.tsx L79 (Browse files, outline/sm), focusedCommitDiffPanel.tsx L156 (Exit diff view, ghost/icon X), codePreviewPanel.tsx L105 (Close preview, ghost/icon X).
Suggested fix:
Add `asChild` to the three `<TooltipTrigger>` elements in fullCommitDiffPanel.tsx (L79), focusedCommitDiffPanel.tsx (L156), and codePreviewPanel.tsx (L105) so each tooltip merges into its single Button/Link, matching CommitActionLink.
Screenshots:
- Before: https://drive.orianna.ai/9d6fdcaaff008e3ddddc839601eb6ce8.png
- After: https://drive.orianna.ai/d2ffe923b204bf22895bfc8d3114edfd.png
Use these screenshot URLs as visual evidence if your environment can open remote images.
How can I resolve this? Keep the fix scoped to the PR-touched UI and preserve existing design-system patterns.
| pathType: 'commit', | ||
| commitSha: parent, | ||
| })} | ||
| className="underline hover:text-foreground" |
There was a problem hiding this comment.
Parent commit links stay underlined at rest
In the commit details line, the parent commit links are always underlined, while every other commit and revision link in the file browser only underlines when you hover. Matching the hover-only style would make these links feel consistent and a little less busy in the dense metadata row.
Prompt to fix with AI
This is a comment left during a design review.
Comment:
**Parent commit links stay underlined at rest**
In the commit details line, the parent commit links are always underlined, while every other commit and revision link in the file browser only underlines when you hover. Matching the hover-only style would make these links feel consistent and a little less busy in the dense metadata row.
Issue:
commitHashLine.tsx L42: parent links use `className="underline hover:text-foreground"` — underline is always on and the only hover affordance is a color change.
Suggested fix:
Change the parent link className in commitHashLine.tsx from `underline hover:text-foreground` to `hover:underline` (optionally keeping the muted color), matching the historyRow and pathHeader SHA links.
Screenshots:
- Before: https://drive.orianna.ai/ea54dfe382fef731164751c0557c09da.png
- After: https://drive.orianna.ai/3201e26f79e7cbccfe50d320b0ee8b94.png
Use these screenshot URLs as visual evidence if your environment can open remote images.
How can I resolve this? Keep the fix scoped to the PR-touched UI and preserve existing design-system patterns.
| } | ||
|
|
||
| const filled = filledSquaresForTotal(total); | ||
| const greenCount = Math.round((filled * additions) / total); |
There was a problem hiding this comment.
Change dots can hide that a file also had deletions
On a file with one line added and one line removed, the little change-indicator squares show only the 'added' color, so at a glance the file looks purely additive even though its own +/- count clearly shows a removed line. The squares should agree with the count beside them.
Prompt to fix with AI
This is a comment left during a design review.
Comment:
**Change dots can hide that a file also had deletions**
On a file with one line added and one line removed, the little change-indicator squares show only the 'added' color, so at a glance the file looks purely additive even though its own +/- count clearly shows a removed line. The squares should agree with the count beside them.
Issue:
DiffStat computes greenCount = Math.round((filled * additions) / total) and redCount = filled - greenCount. For a file with 1 addition and 1 deletion, filledSquaresForTotal(2) = 1 and Math.round(1*1/2) = 1, so greenCount = 1, redCount = 0, and the single filled square renders green (bg-green-500) with no red square, even though the same row prints '+1 -1'.
Suggested fix:
Allocate at least one square to each non-zero side before distributing the remainder (e.g. start greenCount/redCount at 1 when additions/deletions > 0 and filled permits, then split the leftover proportionally), so a file with deletions never shows an all-green single-square indicator.
Screenshots:
- Before: https://drive.orianna.ai/30c9f189aa320beeda74aed1c7405a05.png
- After: https://drive.orianna.ai/451d723bf71f0ed0729ceae48bf645be.png
Use these screenshot URLs as visual evidence if your environment can open remote images.
How can I resolve this? Keep the fix scoped to the PR-touched UI and preserve existing design-system patterns.
| parents={commitResponse.parents} | ||
| /> | ||
| </div> | ||
| <div className="flex flex-row items-center justify-between gap-2 px-4 py-2 border-b shrink-0"> |
There was a problem hiding this comment.
Files-changed summary sits slightly out of line with the diff list
On the commit page, the "N files changed" summary is nudged in a few pixels more than the commit title above it and the file rows below it, so the left edge looks subtly ragged as you scan down the page. Aligning it with the file list makes the commit page read as one clean, deliberate column.
Prompt to fix with AI
This is a comment left during a design review.
Comment:
**Files-changed summary sits slightly out of line with the diff list**
On the commit page, the "N files changed" summary is nudged in a few pixels more than the commit title above it and the file rows below it, so the left edge looks subtly ragged as you scan down the page. Aligning it with the file list makes the commit page read as one clean, deliberate column.
Issue:
The 'N files changed' summary bar uses px-4 (16px) horizontal padding, but the commit header above it uses p-3 (12px) and every FileDiffRow header below it uses py-2 px-3 (12px). The bar's content is therefore inset 4px on both the left and right relative to the column it labels, so 'N files changed' and its diffstat don't line up with the commit subject above or the file paths/diffstats below. The left edge of the page reads as subtly ragged when scanning down.
Suggested fix:
Change the summary bar's horizontal padding from `px-4` to `px-3` so its left and right edges align with the commit header (`p-3`) above and the file diff rows (`px-3`) below, giving the page a single consistent content column.
Screenshots:
- Before: https://drive.orianna.ai/da7868f814871b0f56cce1fe38c8d2c7.png
- After: https://drive.orianna.ai/c79365fdec2d33bdcb5d4ac74fa7f057.png
Use these screenshot URLs as visual evidence if your environment can open remote images.
How can I resolve this? Keep the fix scoped to the PR-touched UI and preserve existing design-system patterns.
| )} | ||
| </div> | ||
| {hasBody && isBodyExpanded && ( | ||
| <CommitBody body={body} className="rounded max-h-[40vh] overflow-y-auto" /> |
There was a problem hiding this comment.
Expanded commit description hugs the title with no breathing room
When you expand the commit description on the commit page, the description panel butts directly against the commit title with no separation, while the author and commit lines below it have clear spacing. A small gap above the description matches the surrounding rhythm and makes the expanded state feel balanced rather than crowded.
Prompt to fix with AI
This is a comment left during a design review.
Comment:
**Expanded commit description hugs the title with no breathing room**
When you expand the commit description on the commit page, the description panel butts directly against the commit title with no separation, while the author and commit lines below it have clear spacing. A small gap above the description matches the surrounding rhythm and makes the expanded state feel balanced rather than crowded.
Issue:
When the commit description is expanded, the rounded, tinted CommitBody panel sits flush against the bottom of the commit subject (h1) with 0px separation, while the author row and hash row below it are separated by the header's gap-2 (8px). The expanded body is nested inside the subject row, so it bypasses the header's vertical gap, producing an uneven rhythm: the description card touches the title above but has space below it.
Suggested fix:
Add `mt-2` to the expanded CommitBody className in commitMessage.tsx (`"mt-2 rounded max-h-[40vh] overflow-y-auto"`) so the description panel is separated from the subject by the same 8px the header already uses between its other stacked rows. This is scoped to the commit page; the commit-history-row usage passes its own className and is unaffected.
Screenshots:
- Before: https://drive.orianna.ai/5a5da0ebe9e8c273cab2e15869124637.png
- After: https://drive.orianna.ai/9b9a505e15f63f8f6806f86d059e6fb2.png
Use these screenshot URLs as visual evidence if your environment can open remote images.
How can I resolve this? Keep the fix scoped to the PR-touched UI and preserve existing design-system patterns.











































Softlight Overview
Design Score: 2/5
Suggested fixes
Redesign
The commit diff screens felt dense and noisy; this pass adds breathing room and a clear hierarchy so the actual changes stand out.
Replacement Design Doctor reasoning-effort A/B clone for #2.
Variant:
xhigh.This branch adds a no-op commit on top of the original PR head so the file contents are equivalent while GitHub checks stay isolated to this test PR.
Replaces contaminated setup PR: #25.