Skip to content

Add a repository-wide commit graph - #384

Open
0x92 wants to merge 5 commits into
dcouple:mainfrom
0x92:feature/commit-graph
Open

Add a repository-wide commit graph#384
0x92 wants to merge 5 commits into
dcouple:mainfrom
0x92:feature/commit-graph

Conversation

@0x92

@0x92 0x92 commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

Description

Pane could show a session's own commits, but never the repository as a whole —
which branch a session came from, what is on main, where a tag sits. This adds a
project-scoped graph of every branch and tag, with Pane's own worktrees marked.

  • Lane diagram solved client-side from a flat commit list, so the layout is
    unit-testable and independent of git log --graph ASCII art
  • Branch starts, joins and crossing lanes are distinct edge kinds; merges are
    drawn hollow
  • Filter by subject, author, hash or ref; focus one branch's history; arrow keys,
    / and Escape to navigate
  • Pane worktrees, uncommitted work and ahead/behind counts tie the graph back to
    the sessions
  • Selecting a commit loads its full patch into the same DiffViewer the diff
    panel uses

The remote scope defaults to the repository's own remote rather than --all: a
fork's clone carries origin and upstream, and those are two different
repositories on the hosting side.

Type of Change

  • New feature (non-breaking change which adds functionality)

Checklist

  • I have read the CONTRIBUTING.md guidelines
  • My code follows the code style of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • My changes generate no new warnings
  • New and existing unit tests pass locally with my changes
  • I have run pnpm typecheck and pnpm lint locally
  • I have tested the Electron app locally with pnpm electron-dev
  • I have added tests that prove my fix is effective or that my feature works

Critical Areas Modified

  • State management/IPC events (projects:get-git-graph,
    projects:get-commit-detail, both through the shared registry)

Screenshots (if applicable)

image

Additional Notes

Tests: layout solver (13), graph manager and ref parsing (28), including
remote-scope resolution and rejection of ref names that could reach a shell.

frontend/src/utils/parseUnifiedDiff.ts also appears in the "commit file
details" PR — same file, same content, needed by both. Whichever merges first,
the other should merge cleanly.

These four feature PRs are independent but all add an entry to the same
navigation plumbing (navigationStore's ActiveView, the two sidebar
components, preload.ts, api.ts, electron.d.ts). Whichever lands first,
the others need a small rebase there — no logic overlaps.

Not done: the packaged-build check from CONTRIBUTING. I develop on Windows,
so pnpm build:mac was not run.

Automated UI QA

Status: passed on b0adf343 with synthetic repository/commit fixtures.

  • Expanded and compact sidebar navigation open the repository commit graph.
  • Graph lanes, refs, worktree marker, selected commit detail, unified diff, free-text filtering, and ArrowDown selection were verified.
  • tests/commit-graph.spec.ts: 1/1 passed.
  • Root pnpm typecheck and pnpm lint: passed.
Default graph and commit detail Filtered history
Repository commit graph with selected commit detail Repository commit graph filtered to one commit

Remaining human check: visual judgment with a large real repository and many concurrent lanes.

Pane could show a session's own commits, but never the repository as a
whole — which branch a session came from, what is on main, where a tag
sits. This adds a project-scoped graph view of every branch and tag, with
Pane's own worktrees marked.

- Lane diagram solved client-side from a flat commit list, so the layout is
  unit-testable and independent of `git log --graph` ASCII art
- Branch starts, joins and crossing lanes are distinct edge kinds; merges
  are drawn hollow
- Filter by subject, author, hash or ref; focus one branch's history;
  arrow keys, `/` and Escape to navigate
- Pane worktrees, uncommitted work and ahead/behind counts tie the graph
  back to the sessions
- Remote scope defaults to the repo's own remote: `--all` mixes a fork's
  `upstream` branches into what should be one project's history

Commit selection loads the full patch into the same DiffViewer the diff
panel uses.

Tests: layout solver (13), graph manager and ref parsing (28), including
remote-scope resolution and rejection of ref names that could reach a shell.
@parsakhaz
parsakhaz force-pushed the feature/commit-graph branch from e917683 to d157fd1 Compare August 23, 2026 19:19

@parsakhaz parsakhaz left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Verdict: Request changes — the rebased feature has a compile failure and two last-mile navigation/state defects.
Counts: Must Fix: 3 (security: 0) · Should Fix: 1 · pass 1/1

Must Fix

  • MF-1 — The new project graph handler references databaseService without taking it from services · main/src/ipc/git.ts:475 · add it to the handler’s service destructuring · violates the PR checklist claim that pnpm typecheck passes.
    • Evidence: pnpm typecheck fails with TS2304: Cannot find name 'databaseService', followed by inferred-type errors at lines 478-490.
    • Failure scenario: The main process cannot compile, so the PR cannot build or ship.
  • MF-2 — The commit graph is absent from the compact sidebar · frontend/src/components/Sidebar.tsx:619 · add a compact-rail graph control and active state for each repository · violates the PR’s stated navigation requirement that a new ActiveView has an entry in both sidebar components.
    • Evidence: The diff adds navigateToGitGraph only to ProjectSessionList; the compact repository rail still exposes only navigateToProject at Sidebar.tsx:637.
    • Failure scenario: A user with the sidebar collapsed cannot open the feature from that navigation surface.
  • MF-3 — Repository-scoped graph state survives a project change, and a focus ref is accepted without proving it exists · frontend/src/components/SessionView.tsx:1621, main/src/services/gitGraphManager.ts:149 · remount/reset graph state by project and ignore unresolved focus refs before git log · violates the project-scoped graph and focus-history intent.
    • Evidence: GitGraphView stores selectedHash, remoteScope, and focusRef in component state, but SessionView reuses the same component instance when only projectId changes. The backend checks only ref syntax before interpolating it into git log; it does not perform the shared type’s documented repository-existence check.
    • Failure scenario: Opening repository B after focusing a branch in repository A can run git log <A-only-ref> in B and replace the graph with an error. Without focus, the old selected hash remains and B’s detail pane stays empty.

Should Fix

  • SF-1 — Worktree matching lowercases paths on every platform · main/src/ipc/git.ts:63 · only case-fold on Windows.
    • Evidence: POSIX filesystems may contain distinct /repo/Foo and /repo/foo paths, but both normalize to the same map key.

Praise

  • The new command surfaces constrain commit hashes, remote names, and focus-ref characters before composing git commands, and the manager tests exercise shell-metacharacter rejection.

⚠️ Cannot verify

  • The repository-local role file required by the code-reviewer skill is absent at .claude/agents/code-reviewer.md; this pass used the canonical user-level role file at ~/.claude/agents/code-reviewer.md with the required report format.

Checks run: git diff --check origin/main...HEAD passed; frontend Vitest passed 285/285; main Vitest passed 651/651; frontend typecheck passed; root typecheck failed on MF-1.

@parsakhaz parsakhaz left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

All three Must-Fix findings are resolved. Root typecheck and lint pass, graph and registry tests pass, and the new commit-graph Playwright journey passes on the pushed head.

@parsakhaz

Copy link
Copy Markdown
Member

Three-pass review complete

Pushed head: b0adf343

Review

  • Fixed the missing database service wiring in the git IPC handler.
  • Added commit graph navigation to the compact sidebar.
  • Scoped graph state to the active project and verified focus refs before use.
  • Preserved case-sensitive paths on POSIX while retaining Windows normalization.
  • Added regression coverage for stale and unknown focus refs.
  • Commit: 68121f13 fix(review): repair commit graph wiring and scope

Simplify

  • Tightened request and command-runner contracts and removed conditional object construction where it obscured control flow.
  • Centralized the graph limit and shared request types.
  • Removed dead/default exports and renderer assertions that were no longer needed.
  • Reduced test indirection through a narrow command-runner seam.
  • Commit: 3e5d8c67 refactor(simplify): tighten commit graph contracts
  • Result: typecheck and targeted tests stayed green, while root lint improved from 23 PR-local anti-slop findings plus dead exports to clean.

Refactor

  • Centralized shared diff result types used by main and renderer.
  • Removed the duplicate unified diff parser from the graph view and reused the tested renderer utility.
  • Added a runtime decoder at the IPC boundary for graph requests, including rejection of invalid project IDs and non-integer limits.
  • Added boundary tests.
  • Commit: 3f73c924 refactor: centralize commit graph boundaries

QA fix

  • Replaced nested commit-row buttons with sibling controls so ref chips are valid, independently interactive elements.
  • Added a Playwright flow for expanded and compact sidebar navigation, graph lanes and refs, worktree state, commit details, unified diff, filtering, and keyboard selection.
  • Commit: b0adf343 fix(qa): avoid nested commit graph controls
Default graph Filtered graph
Commit graph default Commit graph filtered

Verification

  • pnpm typecheck: passed
  • pnpm lint: passed
  • Frontend Vitest suite: 285 passed
  • Main Vitest suite: 651 passed
  • Simplify comparison: frontend 22 of 22 and main 40 of 40 passed before and after
  • Deep refactor targets: frontend 19 of 19 and main 42 of 42 passed
  • pnpm build:main: passed during isolated app launch
  • pnpm exec playwright test tests/commit-graph.spec.ts: 1 passed

Follow-ups

  1. Split the roughly 800-line GitGraphView into a controller hook, list, and row components after component-level integration coverage exists.
  2. Filter generic panel:event refreshes to completed git operations for sessions in the current project after event integration coverage exists.
  3. Audit whether top-level RepoGitGraph.refs can be removed because the renderer currently consumes node-level refs.

Left for parsa

  • Perform the final human visual check with a large repository and many simultaneous graph lanes.
  • Merge when satisfied. No merge was performed.

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