Skip to content

🤖 fix: diff old side against the merge base - #28

Closed
tskimmett wants to merge 104 commits into
coder:mainfrom
tskimmett:fix/diff-merge-base
Closed

🤖 fix: diff old side against the merge base#28
tskimmett wants to merge 104 commits into
coder:mainfrom
tskimmett:fix/diff-merge-base

Conversation

@tskimmett

@tskimmett tskimmett commented Oct 9, 2026

Copy link
Copy Markdown

Deleted lines could show unrelated code once full file contents loaded (they looked correct for a moment from the patch-only render first).

  • Cause: the old side was fetched at `pr.base.sha` (base branch tip), but GitHub's patches are against the merge base. After the base branch moves, old line numbers point at different code. Now resolved through `getMergeBase` (one compare call, cached persistently by SHA pair). The range view ("changes since") is unchanged since it already used the start commit.
  • Guard: the worker only uses a pre-highlighted full-file line when its raw text matches the patch line, falling back to highlighting the patch text.
  • Gap context: revealed context lines (default and manual expansion) used the new line number as the old one. `gapLineOffsets` computes the per-gap drift.

tskimmett and others added 30 commits August 25, 2026 15:49
@tskimmett @claude
chore: refresh bun.lock integrity hash
d2d8e9c 
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@tskimmett @claude
docs: add semantic review spec
3e1e0d2 
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@tskimmett @claude
feat: add hide-test-files toggle to file panel
90478bb 
Heuristic path-based classification (src/browser/lib/test-file.ts) with a
persisted global preference. Hidden test files are removed from the file
tree, j/k navigation, and the command palette, with a count indicator.

Part of the semantic review work (docs/semantic-review.md), but useful
standalone for cutting cognitive load on large PRs.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@tskimmett @claude
feat: add semantic review data model, validation, and coverage reconc…
9145747 
…iliation

Pure data layer for semantic review (docs/semantic-review.md):
- schema.ts: SemanticReview types + structural validator that collects all
  errors for provider self-correction
- patch.ts: minimal GitHub patch-string hunk parser for coverage math
- coverage.ts: enforces the coverage invariant - prunes ranges matching no
  real hunk and sweeps uncovered hunks into a synthetic cohort

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@tskimmett @claude
feat: add semantic review local server (providers, jobs, cache, SSE r…
40c7362 
…outes)

- Pluggable providers: Claude (Agent SDK) and Codex (CLI), both run
  prompt -> raw text; availability detected from local credentials/PATH
- Prompt builder with lockfile/minified patch elision and size capping,
  plus JSON extraction and a one-shot self-correction prompt
- Job runner: validate -> correct -> reconcile coverage -> disk cache
  (~/.pulldash/semantic, keyed by PR + head SHA)
- Hono routes under /api/semantic with SSE job progress streaming;
  self-disables on hosted (Vercel) deployments

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@tskimmett @claude
feat: add semantic review browser state and trigger button
b180404 
- Semantic slice in PRReviewStore: provider list, cached-result load,
  analysis job lifecycle (SSE progress, cancel, dispose), view mode,
  per-PR reviewed-layer persistence
- semantic-client.ts: fetch/EventSource wrappers that degrade to a
  hidden feature when no local providers exist (hosted deployment)
- SemanticReviewButton in the PR header: provider picker, live progress
  dropdown with cancel, error/retry, and a Files/Semantic view toggle

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@tskimmett @claude
feat: add semantic review mode UI (cohort/layer nav, layer bar, keybo…
5e1212f 
…ard)

- SemanticSidebar replaces the file tree in semantic view mode:
  cohort -> layer tree with reviewed marks, staleness banner, overview,
  and a layers-reviewed progress footer
- SemanticLayerBar above the diff: cohort > layer breadcrumb, markdown
  summary, clickable range chips that jump to file + line, and a
  mark-reviewed control
- Store: layer navigation/selection, range jumping via the existing
  selectFile + focusedLine machinery, one-way viewed-file sync when all
  of a file's ranges sit in reviewed layers
- Keyboard: j/k move between layers and v marks a layer reviewed while
  in semantic mode

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@tskimmett @claude
feat: add lazy-loaded mermaid diagrams to semantic layers
9df8a86 
- MermaidDiagram component: dynamic import of mermaid (dark theme,
  strict security), raw-source code block fallback on invalid syntax
- Collapsed-by-default 'Show diagram' toggle in the layer bar so the
  library only loads when a diagram is actually opened
- Enable code splitting in the browser build; mermaid lands in lazy
  chunks and the main bundle stays at its previous size

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@tskimmett @claude
fix: black screen from wrong entry chunk in split builds
89078c5 
With splitting enabled, Bun's HTML entrypoint rewriting can point the
<script> tag at an arbitrary chunk (here: a mermaid sub-chunk) instead
of the real entry point, so the app never mounted. Rewrite the script
src from the build manifest, which reports the entry point correctly,
and clean dist/browser before each build so stale chunks from previous
hash assignments can't be served.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@tskimmett @claude
refactor: use @openai/codex-sdk for the Codex provider
656675e 
Replace hand-rolled `codex exec` spawning with the official TypeScript
SDK, which bundles the codex binary and exchanges typed JSONL events.
Availability is now gated on credentials (ChatGPT login or API key)
instead of a PATH lookup, mirroring the Claude provider, and reasoning
summaries stream into the job progress log instead of a byte-count
heuristic.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@tskimmett @claude
fix: keep semantic analysis prompts inside provider context windows
2c9ee36 
Claude Code rejected large PRs with "Prompt is too long": the shared
600k-char ceiling overshoots its 200k-token window once diff
tokenization (~3 chars/token) and the system prompt are accounted for.
Give providers a per-provider prompt budget (Claude: 250k chars) and,
if a provider still rejects the prompt as too long, halve the budget
and retry with the largest patches elided instead of failing the job.
The correction round-trip goes through the same shrink loop.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@tskimmett @claude
feat: map-reduce analysis for PRs too large for one prompt
603ce85 
Eliding patches to fit a big PR into a single prompt discards exactly
the changes that most need review. Instead, when the full diff exceeds
the provider's prompt budget, partition it into sections that fit, run
a map pass annotating each section's hunks with semantic fragments
(range + label + summary), then a reduce pass that organizes all
fragments into cohorts/layers without re-reading patches. Coverage
reconciliation still sweeps anything missed into "Uncovered changes".
Small PRs keep the cheaper single-shot path.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@tskimmett @claude
fix: recognize .unit.* files and .NET Tests/ folders as test files
c77e852 
The hide-test-files heuristic missed common conventions: *.unit.ts
basenames, capitalized test dirs (Tests/, Test/), and .NET test-project
folders (MyProject.Tests/, MyProject.UnitTests/). Files matching those
conventions were neither counted nor hidden, which made the toggle look
like a no-op in repos using them.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@tskimmett @claude
fix: surface expired Claude Code login as an actionable error
b4a7edb 
The Agent SDK reports auth failures as a result message, which we were
returning as analysis output. Detect them and tell the user to run
`claude login` instead of failing later in JSON extraction.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@tskimmett @claude
feat: show which model each provider uses in progress logs
35a47e3 
Claude: taken from the Agent SDK's init system message. Codex: the SDK's
events never report the model, so read the configured default from
~/.codex/config.toml (best-effort).

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@tskimmett @claude
fix: sync semantic layer selection when navigating files
8442510 
In semantic mode, file-level navigation (file-header prev/next arrows,
j/k unviewed-file navigation, file tree clicks, hash navigation) all
route through selectFile, which never touched selectedLayerId. Landing
on a file belonging to a different layer left the semantic sidebar and
layer bar showing the stale layer.

selectFile now resolves the first layer (in cohort/layer order) whose
ranges cover the target file and selects it. The resolver returns the
currently selected layer when that layer already covers the file, so the
layer -> file jump in selectSemanticLayer is a no-op and an intentional
layer selection is never clobbered by an overlapping earlier layer.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@tskimmett @claude
feat: add light mode with light/dark/system theme toggle
f556b12 
Splits the single dark palette in index.css into a light `:root` default and
a `.dark` override, and tokenizes the theme-dependent values that previously
bypassed CSS variables: scrollbars, markdown surfaces, the highlight.js
palette (GitHub Light/Dark), diff line-selection colors, and the diff
add/remove/comment line backgrounds that pr-review.tsx applied as inline hex.

Theme state lives in a standalone module (lib/theme.ts) persisted under
`pulldash_theme`, following the existing localStorage preference pattern.
"system" tracks prefers-color-scheme and reacts to changes. A pre-paint
inline script in index.html sets the class before first paint so there is no
flash, replacing the hardcoded `class="dark"`.

Mermaid previously memoized `initialize({ theme: "dark" })` for the process
lifetime; it now re-initializes per render and the effect depends on the
resolved theme, since mermaid bakes colors into the emitted SVG.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@tskimmett @claude
feat: collapse and expand inline diff comments
a4b7933 
Adds a global collapse-all toggle in the diff file header plus a
per-thread collapse control. Collapse state is modeled as a persisted
global default with in-memory per-thread overrides, so collapse-all and
manual per-thread toggles compose. Collapsed threads render a one-line
stub (author + preview) and auto-expand while focused, edited, or being
replied to so navigation and unsaved input are never hidden.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@tskimmett @claude
feat: make the sidebar width resizable by dragging its right edge
73ab6aa 
Shared between the file tree and semantic sidebar, persisted to
localStorage, clamped to 200-520px, double-click resets to default.
Desktop only; the mobile drawer keeps the stored width capped at 85vw.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@tskimmett @claude
Improve contrast: dark diff line colors, brighter range chips, larger…
6861760 
… sidebar text

- Add .dark overrides for the four --diff-line-*-bg tokens (dark diffs
  were rendering the light-mode red/green backgrounds)
- Range chips: readable selected pill in light mode (solid violet +
  white text) and brighter default pills with a border
- Semantic sidebar: bump cohort headers and layer rows to 13px

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@tskimmett @claude
Replace Geist with native system font stacks
3214145 
Drops the geist package and self-hosted @font-face declarations; UI
text uses the platform sans stack and code uses ui-monospace.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@tskimmett @claude
GitHub-style incremental diff gap expansion
1910c20 
Skip blocks now expand 20 lines at a time from either edge instead of
all at once:
- Store tracks {top, bottom} revealed lines per gap; the remaining
  collapsed portion stays rendered as a (shrinking) skip row
- Expander gutter gets directional buttons: expand-up extends the hunk
  below, expand-down extends the hunk above, and gaps of 20 lines or
  fewer show a single expand-all button; top-of-file gaps only offer up
- Enter on a focused skip block still expands the whole remaining gap
- Keyboard navigation (unified + split) walks partially expanded gaps

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@tskimmett @claude
Add expand-entire-file button to the diff header
a5699e1 
Dispatches an expand-all event that reveals every remaining gap in the
current file; the underlying file fetch is cached so it downloads once.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@tskimmett @claude
Allow expanding the diff below the last change
1af2bfd 
Diffs can't tell whether a file continues past the last hunk, so the
viewer now synthesizes an end-of-file gap row (skipped for added and
removed files). Its size starts unknown; the first expansion fetches
the file, records its true line count in the store, and clamps all gap
expansion to EOF. If the last hunk already reaches EOF the row
disappears after that first click. The header expand-all button covers
the trailing gap too.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@tskimmett @claude
Show 20 lines of context around each change by default
d2999a6 
The diff worker already receives the full head-file content for
highlighting, so it now also emits 20 pre-highlighted context lines on
each side of every gap (plus the file's total line count) at parse
time. setLoadedDiff seeds these into the expansion state, so gaps
narrower than 40 lines are fully open by default, the end-of-file gap
shows an exact count immediately, and the incremental expanders pick up
from there. User expansions are never overwritten.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@tskimmett @claude
Reduce diff font size from 0.8rem to 0.75rem
ebc69ab 
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@tskimmett @claude
Match GitHub's syntax highlighting colors
2c8cd13 
Replace the One Light/One Dark prism themes with GitHub's Primer
palette: red keywords/operators, purple functions, blue constants,
dark-blue strings, green tags, orange special variables - mapped for
both light (github.com light) and dark (GitHub Dark Default) modes.
Drops unused prism plugin rules (previewers, line numbers).

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@tskimmett @claude
Use Monaspace Neon for diffs and code
d8d1379 
Self-hosted from @fontsource/monaspace-neon (400/400i/700), prepended
to the mono font stack.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@tskimmett @claude
Match github.com's monospace stack for diffs
7218288 
Monaspace Neon is a GitHub-made font but github.com itself renders code
in its classic ui-monospace/SF Mono/Consolas stack, so the diff looked
different from the site. Drop the bundled font entirely.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@tskimmett @claude
Turn expand-entire-file button into a collapse toggle
3a3eb76 
After expanding a file the header button becomes a fold icon that
resets the file's gaps to the default 20-line seeded context.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
tskimmett and others added 27 commits October 2, 2026 10:44
@tskimmett @claude
feat: highlight matched text within lines when finding in diff
85afd4b 
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@tskimmett @claude
feat: search PRs by text from the header input, scoped to the feed
211d00b 
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@tskimmett @claude
feat: two-stage PR search that also finds non-feed PRs in feed repos
a2f017e 
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@tskimmett @claude
feat: repeat file header below each diff in All Files view
0cd1e9f 
Also add more space between files and a slightly stronger border.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@tskimmett @claude
feat: only show diff footer bar for longer diffs
2b5d1c3 
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@tskimmett @claude
fix: re-measure mounted rows so viewed toggles don't overlap files in…
55191d8 
… All Files

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@tskimmett @claude
feat: recursive expand/collapse in file tree folder context menu
d97c0d0 
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@tskimmett @claude
fix: stop clearing virtualizer measurements on viewed toggle in All F…
8e8500e 
…iles

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@tskimmett @claude
feat: find PRs by pasted branch name in header search
9c155f0 
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@tskimmett @claude
feat: schedule auto-merge from the PR merge box
fd8ae05 
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@tskimmett @claude
fix: only scroll to find match on explicit find navigation
1980d7a 
Expanding context or inserting comment rows shifted the active match's
row index, re-triggering scrollToIndex and yanking the diff viewer while
scrolling.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@tskimmett @claude
feat: show diff stat of visible files next to hidden test files count
325b4a4 
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@tskimmett @claude
refactor: rename to better pr and remove upstream pulldash cruft
beb3970 
- Remove Vercel deploy (vercel.json, build script, src/index.ts entry)
- Remove Electron app, release workflow, and icon generation
- Remove PostHog telemetry (reported to upstream's project key)
- Remove GitHub device-flow login (used upstream's OAuth app); PAT only
- Remove demo content: sample PRs, marketing animation, anonymous mode,
  and read-only banners/prompts that only applied to anonymous mode
- Remove Next.js/eslint leftovers, unused setup-mux action, screenshots
- Rename branding to "better pr"; semantic cache dir is now ~/.better-pr
  (BETTER_PR_SEMANTIC_CACHE_DIR). localStorage keys keep the pulldash_
  prefix so existing sessions and settings survive.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@tskimmett @claude
feat: deploy browser build to Azure Static Web Apps
4f58608 
Adds a staticwebapp.config.json (SPA fallback excluding assets and
/api/*, immutable caching for hashed chunks) copied into dist/browser by
the build, and `bun run deploy`, which builds and uploads via the SWA CLI.
Target subscription/resource group/app name come from env (.env) so no
org-specific values live in the repo.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@tskimmett @claude
fix: harden token handling, local server, and AI agent sandboxing
edcb8e1 
- Bind the local server to 127.0.0.1 and reject /api requests with a
  non-loopback Host, cross-site Origin/Sec-Fetch-Site, or non-JSON POST
- Run Claude and Codex agents without tools, user settings, or MCP
  servers, in an empty scratch dir with a minimal env; mark PR content
  as untrusted in prompts
- Add a CSP (inline script allowed by build-time hash) to the Static Web
  App config and the local server; make the bookmarklet CSP-compatible
- Filter tags/attributes in GitHub-rendered HTML; drop images from
  model-generated markdown
- Never delete/restore branches for fork PRs (hit the base repo)
- Accept fine-grained PATs; require full repo scope for classic PATs;
  correct the token storage text
- Pin the SWA CLI, CI actions, and Bun; frozen lockfile and read-only
  permissions in CI; drop stale coder/mux script references

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@tskimmett @claude
fix: responsive layout for mobile and narrow viewports
3184020 
- Overview no longer scrolls horizontally: markdown editor toolbar wraps,
  long branch/check/path/user strings wrap, skeleton matches real layout
- File header wraps its toolbar onto a second row instead of overlapping
- PR header and toolbar collapse labels to icons below md
- Mobile tab switcher dropdown replaces the tab strip below sm
- PR search reachable on phones via a full-width row
- Split diffs render unified below md without changing the saved setting
- Keyboard shortcut hints hidden on coarse pointers (kbd,
  [data-keyboard-hint], menu shortcut slots)
- Fixed-position popovers and menus clamped to the viewport
- h-dvh shell, 16px inputs on phones to avoid iOS focus zoom
- Real xs breakpoint via @theme instead of hand-written media rules

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@tskimmett @claude
feat: scroll long diff lines horizontally on mobile instead of wrapping
206b9a9 
Below md, diff lines use white-space: pre inside a horizontal scroller.
The virtualized single-file view sizes its container from the widest
line (estimated in ch from the highlighted HTML, erring wide); the
all-files view lets each file's lines size to max-content. Comments,
comment forms and skip rows stay pinned to the visible width.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@tskimmett @claude
fix: use chevron icon and equal heights in split buttons
f7538aa 
The "▾" glyph rendered oversized and off-center on Android and slightly
misaligned elsewhere. Swap it for lucide's ChevronDown and stretch both
halves of the diff-range and semantic split buttons to the same height.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@tskimmett @claude
feat: update welcome dialog tagline
2f23d04 
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@tskimmett @claude
fix: separate tab strip from header search box
76ae17b 
Fade the overflowing tab strip, keep the right-hand controls from
shrinking, and narrow the search box below the lg breakpoint.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@tskimmett @claude
refactor: extract hunk line computation from diff worker
af5090d 
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@tskimmett @claude
feat: PR stack navigation; fix commits/checks tabs on mobile
79d4f6c 
Hide the overview sidebar on mobile unless the Conversation tab is active so
the commits/checks lists aren't pushed off-screen.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@tskimmett @claude
feat: add What's new dialog with changelog
deadfe6 
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@tskimmett @claude
perf: show PR stack nav immediately from cached stacks
6c8f7ba 
Seed the stack cache from the PR list's discovery, persist stacks, and
render the last known stack while revalidating. Also limit changelog
entries to new features and notable behavior changes.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@tskimmett @claude
feat: limit open PR tabs to 5, dropping the oldest
45feb8b 
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@tskimmett @claude
feat: show full mode label with repo below in dashboard filter chips
990319d 
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@tskimmett @claude
fix: diff old side against the merge base, not the base branch tip
05de12b 
Full-file highlighting replaced deleted lines with content fetched at
pr.base.sha, which drifts from GitHub's merge-base patch once the base
branch moves. Fetch the old side at the merge base, and only use a
pre-highlighted line when its text matches the patch.

Also give revealed gap context lines correct old line numbers instead
of reusing the new ones.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@vercel

vercel Bot commented Oct 9, 2026

Copy link
Copy Markdown

@tskimmett is attempting to deploy a commit to the Coder Team on Vercel.

A member of the Team first needs to authorize it.

@tskimmett tskimmett closed this Oct 9, 2026
@tskimmett
tskimmett deleted the fix/diff-merge-base branch October 9, 2026 14:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@tskimmett