Skip to content

[design-doctor test] sourcebot-1154: Commit diff viewer#2

Open
adiraju13 wants to merge 14 commits into
bench/source-pr-base/design-doctor-feature-parity-expanded-20260610-152904Z/sourcebot-1154from
bench/design-doctor-dev-fresh-20260610-190958Z/sourcebot-1
Open

[design-doctor test] sourcebot-1154: Commit diff viewer#2
adiraju13 wants to merge 14 commits into
bench/source-pr-base/design-doctor-feature-parity-expanded-20260610-152904Z/sourcebot-1154from
bench/design-doctor-dev-fresh-20260610-190958Z/sourcebot-1

Conversation

@adiraju13

@adiraju13 adiraju13 commented Jun 10, 2026

Copy link
Copy Markdown

Softlight Overview

Current PR UI

Design Score: 1/5

Suggested fixes

  • Commit diff code should read at the same size and spacing as the file viewer so it's easy to scan.

Redesign

The commit diff scattered author, date and SHA across cramped lines and rendered code that didn't match the file viewer; the pass folds the details into one tidy line and makes the diff calm and aligned.

  • Commit header now leads with the title and folds author, date, SHA and parent into a single quiet metadata line, with parent links in the app's standard link color.
  • The split diff reads in the same font and gutter as the file viewer, with a quiet empty column so additions stand out instead of sitting next to a grey slab.

Fresh clean test PR for hosted Design Doctor rerun testing.

Benchmark note: Commit diff viewer with full and focused views.

Do not merge. This PR exists so Design Doctor can be run against a clean pull request with no prior review artifacts.

brendan-kellam and others added 14 commits April 28, 2026 10:32
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>
})}
aria-label="Exit diff view"
>
<X className="h-4 w-4" />

This comment was marked as outdated.

})}
aria-label="Close preview"
>
<X className="h-4 w-4" />

This comment was marked as outdated.

<span className="text-sm text-muted-foreground">by</span>
<AuthorsAvatarGroup authors={authors} />
<span
className="text-sm font-medium"

This comment was marked as outdated.

parents={commitResponse.parents}
/>
</div>
<div className="flex flex-row items-center justify-between gap-2 px-4 py-2 border-b shrink-0">

This comment was marked as outdated.

@softlight

softlight Bot commented Jun 10, 2026

Copy link
Copy Markdown

Redesign

Full commit view: one tidy metadata line and a diff that matches the file viewer

Before · After

Full commit view

  • Author, date, commit SHA and parent moved from two scattered mono rows into a single calm metadata line; parent/commit links now use the standard link color.
  • The split diff adopts the file viewer's font and gutter, tinted +/- markers, and a quiet empty column, and the per-file vs. hunk dividers now have clear hierarchy.

Focused single-file view: a header that no longer crams and wraps

Before · After

Focused single-file view

  • The overcrowded one-line header is reorganized into context (status, author, date) on the left and actions (change counts, View full commit, close) on the right, with the author name truncating instead of wrapping.
  • The single-file diff inherits the same file-viewer font, tinted markers and quiet empty column as the full view.
Prompt to build with AI
This is a comment left during a design review.

Goal:
Apply the Designer's pass shown in the before/after references below. Preserve product behavior, routes, data meaning, and the local design system while matching the stronger composition, hierarchy, spacing, alignment, and visual balance.

Current PR screenshots:
- Full commit diff viewer current PR: https://drive.orianna.ai/e4babbbb50e705067411484f06501933.png
- Focused single-file commit diff current PR: https://drive.orianna.ai/6bb67e9d974a30c57eecc6692e360041.png

After/Designer's pass screenshots:
- Full commit diff viewer prototype: https://drive.orianna.ai/1069aa2802f450b3cbe876e00c588e51.png
- Focused single-file commit diff prototype: https://drive.orianna.ai/148c46b1df339fea452007ec47dc5e33.png

Context:
On the commit diff viewer, the original commit header split the same provenance across three different rows, the parent links looked like plain gray text, and the diff used a smaller, mismatched font with a heavy grey block on the empty side of each change. The focused single-file view crammed status, author, date and a navigation link onto one line that wrapped awkwardly on a narrow panel. The pass folds the header into one calm metadata line, makes the diff read like the file viewer, and reorganizes the focused row into clear context and actions.

What the Designer's pass changed:
- Full commit view: the author, date, commit SHA and parent that used to be spread over two cramped rows now sit on one tidy line, and parent/commit links use the app's normal link blue so they read as links.
- Both diff views: code and line numbers now use the same font and size as the file viewer, the +/- markers are tinted, and the empty side of a change is a quiet wash instead of a solid grey block, so additions and deletions stand out.
- Focused single-file view: the overcrowded header line is split into context (file status, author, date) on the left and actions (change counts, View full commit, close) on the right, so it no longer wraps or collides on narrower panels.
- Diff structure: the per-file header and the hunk markers now have distinct weights, so it's easy to tell where one file ends and the next begins.

Use the before screenshots to understand the current surface and the after screenshots as the target direction. Make the smallest coherent set of source changes that gets the existing app to that visual result, then render and inspect the UI before stopping.

parents={commitResponse.parents}
/>
</div>
<div className="flex flex-row items-center justify-between gap-2 px-4 py-2 border-b shrink-0">

This comment was marked as outdated.

})}
aria-label="Exit diff view"
>
<X className="h-4 w-4" />

This comment was marked as outdated.

})}
aria-label="Close preview"
>
<X className="h-4 w-4" />

This comment was marked as outdated.

</div>
{file ? (
<>
<div className="flex flex-row items-center justify-between gap-2 px-4 py-2 border-b shrink-0">

This comment was marked as outdated.

This was referenced Jun 17, 2026
parents={commitResponse.parents}
/>
</div>
<div className="flex flex-row items-center justify-between gap-2 px-4 py-2 border-b shrink-0">

This comment was marked as outdated.

asChild
variant="ghost"
size="icon"
className="h-6 w-6 text-muted-foreground"

This comment was marked as outdated.

parents={commitResponse.parents}
/>
</div>
<div className="flex flex-row items-center justify-between gap-2 px-4 py-2 border-b shrink-0">

This comment was marked as outdated.

<div className="flex flex-row items-center gap-2">
<DiffStat {...computeChangeCounts(file)} />
<Tooltip key={commitSha}>
<TooltipTrigger>

This comment was marked as outdated.


return (
<div
className="font-mono text-xs leading-relaxed"

This comment was marked as outdated.

<CommitMessage subject={subject} body={commitResponse.body} />
</div>
<Tooltip key={commitSha}>
<TooltipTrigger>

This comment was marked as outdated.

path,
pathType: 'blob',
})}
aria-label="Exit diff view"

This comment was marked as outdated.


const filled = filledSquaresForTotal(total);
const greenCount = Math.round((filled * additions) / total);
const redCount = filled - greenCount;

This comment was marked as outdated.


return (
<div
className="font-mono text-xs leading-relaxed"

This comment was marked as outdated.

pathType: 'commit',
commitSha: parent,
})}
className="underline hover:text-foreground"

This comment was marked as outdated.

</Tooltip>
</div>
</div>
<div className="flex-1 min-h-0 overflow-y-auto">

This comment was marked as outdated.

if (!line) {
return (
<div
className={`${SIDE_BG[side].blank} ${separator} px-2 py-px`}

This comment was marked as outdated.


return (
<div
className="font-mono text-xs leading-relaxed"

This comment was marked as outdated.

pathType: 'commit',
commitSha: parent,
})}
className="underline hover:text-foreground"

This comment was marked as outdated.


return (
<div
className="font-mono text-xs leading-relaxed"

This comment was marked as outdated.

pathType: 'commit',
commitSha: parent,
})}
className="underline hover:text-foreground"

This comment was marked as outdated.


return (
<div
className="font-mono text-xs leading-relaxed"

@softlight softlight Bot Jun 26, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Before/after suggested changes

Before · After

Diff code is smaller and looser than the rest of the code browser

The commit diff shows its code and line numbers in a smaller, more spaced-out style than the file viewer, so the diff feels cramped and out of place next to the rest of the code-browsing experience. Matching the file viewer's text size and spacing would make the diff easier to read and feel like one product.

Prompt to fix with AI
This is a comment left during a design review.

Comment:
**Diff code is smaller and looser than the rest of the code browser**
The commit diff shows its code and line numbers in a smaller, more spaced-out style than the file viewer, so the diff feels cramped and out of place next to the rest of the code-browsing experience. Matching the file viewer's text size and spacing would make the diff easier to read and feel like one product.

Issue:
lightweightDiffViewer.tsx:84 sets the diff grid root to 'font-mono text-xs leading-relaxed' (12px, line-height 1.625); line-number spans use text-muted-foreground with px-2. This is the code shown for every file in both the full and focused diff views.

Suggested fix:
Render diff code and line numbers with the file viewer's typography: use the editor font size/family token (font-editor / --editor-font-size 13px, or text-sm to match lightweightCodeHighlighter) and the file viewer's line-height instead of 'text-xs leading-relaxed', and align the gutter padding/color with the editor gutter.

Screenshots:
- Before: https://drive.orianna.ai/61286b00a047d119235ccebb835c76b7.png
- After: https://drive.orianna.ai/413e215840ca60b406498979da78d069.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.

if (!line) {
return (
<div
className={`${SIDE_BG[side].blank} ${separator} px-2 py-px`}

This comment was marked as outdated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants