Skip to content

Preserve Git history lanes through unrelated roots - #26932

Merged
nwparker merged 3 commits into
mainfrom
nwparker/preserve-git-graph-roots-10-09
Oct 10, 2026
Merged

nwparker merged 3 commits into
mainfrom
nwparker/preserve-git-graph-roots-10-09

Conversation

@nwparker

@nwparker nwparker commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor
Files Added Deleted Net
Test 3 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​299 $\color{#cf222e}{\Huge{\mathbf{−}}}$​1 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​298
Prod 2 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​50 $\color{#cf222e}{\Huge{\mathbf{−}}}$​28 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​22

ELI5

When a repository merges two unrelated histories, the Commits graph can lose one history's line and restart it in another color. Keep that line connected until its own history ends.

What Changed

Root commits now end only their own graph lanes. The renderer follows the remaining lanes without skipping an output position, and commit dots and reference labels use the same color rule. Regression tests exercise real unrelated Git repositories and the rendered SVG.

Why

The shared graph model dropped every active lane at a root. Repairing that model and its existing renderer preserves the relationship for local and remote histories without changing Git queries, parsing or history ordering. This also avoids introducing another layout implementation.

Based on the material repair in #25361 by @matbrgz, credited as a co-author with the name and email from the original commits.

Linked Issue

Fixes #25360

Visual Proof

Actual hidden Electron Commits panel, same four-commit repository and renderer bounds. Before: the first history disconnects below the unrelated root and changes color. After: its blue lane continues through that root and the remaining commits.

Before:

Before: first history disconnects below the unrelated root

After:

After: first history remains connected through the unrelated root

Testing

  • I manually tested these changes locally
  • Automated tests added/updated, or explained why not below

Initial macOS validation at 67f9556d4f01dad2e65708aff5bf5eeaabc7c303: actual hidden Electron Commits panel; canonical Bun and native Node each pass the same six suites / 41 tests, including two real Git repositories, parser, allocation, graph, read-budget and SVG checks. Web and Node typechecks, production Electron build, changed-file lint, formatting and all seven changed-code quality gates pass.

Native Windows and Linux: the same six suites / 41 tests pass at 67f9556d4f01dad2e65708aff5bf5eeaabc7c303, including both native Git temporary-repository cases. Windows uses Node 24.18.0 and Git 2.55.0.windows.3; Linux uses Node 26.8.1 and Git 2.55.0. Both use Vitest 5.0.3, their own locked dependency folders, and ORCA_BACKGROUND_LAUNCH=1. These checks cover source and SVG fixtures; actual hidden Electron proof above is macOS only. No live SSH renderer claim; host routing, Git commands and wire formats are unchanged.

Review-bot follow-up at 355e537c0af87f79db06332c1ef4bdaf69bdd3a0: the real-Git fixture clears inherited Git environment variables before applying its own config and dates. Two regressions reproduce repository redirection and config injection using disposable caller repositories, then verify those callers' HEAD, config, index and refs remain unchanged. Both fail before the isolation fix; all 35 tests in four fixture/history suites pass afterward. Node typecheck, full-file lint, formatting and changed-code quality checks pass. This follow-up changes one test file; the production graph and rendered proof remain unchanged.

Integration follow-up at d4a06583cb24c0cc2ade92ac0aadfc457d01d65c: initial new-head CI exposed the original test import removed by the process-package migration on current main (TS2307 during the CLI build). A normal main merge and the existing public @orca/process-host import repair that boundary. The final six suites pass 43 tests, including allocation/read-budget and SVG controls; canonical CLI and Node typechecks, full-file lint/formatting, and all changed-code quality checks pass against the actual five-file PR diff. The original graph production files remain unchanged. The first CI failure is retained; the corrected commit starts ordinary CI without a retry.

AI Disclosure

Review

Independent source/behavior and root final reviews are clean, including the final duplicate-root model and renderer controls.

Agent skill upstream boundary

  • Not applicable, or this change copies or mechanically translates no upstream skill-installer source, tests, fixtures, registry entries, path tables, comments, or documentation.

Notes

No new production Git command or capability requirement. Real-repository tests use the existing cross-platform process wrapper and temporary paths. Existing graph colors and layout dimensions are retained.

Checklist

  • This PR is small and focused
  • I explained what changed and why (ELI5, the user-facing before/after, the mechanism, and why over the alternatives)
  • Before/after screenshots or videos attached for UI changes, or N/A with reason
  • Self-reviewed for correctness, security, and performance
  • Cross-platform, SSH/remote, and path/shortcut impact considered (or N/A)
  • pnpm lint, pnpm typecheck, pnpm test, and pnpm build pass (or CI will cover; local preferred)

Carry unrelated lanes through root commits and share node color selection
between labels and the graph. Keep the renderer's output cursor aligned
when a root has no replacement parent edge.

Cover both root positions, real unrelated Git histories, rendered paths
and colors, truncated history, and existing boundary/read-budget behavior.

Co-authored-by: Matheus Breguêz <matbrgz@gmail.com>
@nwparker
nwparker marked this pull request as ready for review October 9, 2026 18:52
@coderabbitai

coderabbitai Bot commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 73f3129f-fc90-491f-8b43-1eff0961d81b


📥 Commits

Reviewing files that changed from the base of the PR and between 355e537 and d4a0658.



📒 Files selected for processing (1)
  • src/shared/git-history-real-repository.test.ts


Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 6 remain after this review.




📝 Walkthrough
📝 Walkthrough

Walkthrough

The graph model carries active swimlanes through root commits while removing only the lane that ends at that root. It resolves node colors from lane state and exposes getGitHistoryItemColor for SVG rendering. The SVG uses that color resolver and adjusts output-lane indexing for parentless items. Tests cover model behavior, SVG output, and real repositories with unrelated histories.



Priority: ➖ Normal

Severity of issue fixed: Medium

Merge Risk: ⚪ Minimal · up to d4a06

No actionable issue remains in this review scope; the change is mergeable after normal checks.

Pre-merge checks | Passed 4 | Failed 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 10 functions across 5 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check Passed Issue #25360 requires the active lane to continue below an unrelated root. buildGitHistoryViewModels now removes only the lane for the root and preserves other active lanes. The SVG renderer does no…
Out of Scope Changes check Passed The changes stay within issue #25360. Production changes repair the shared graph model and its existing SVG renderer. The added tests verify lane continuity, color behavior, truncated history, and fix…
Title check Passed The title clearly and concisely describes the primary change: preserving Git history lanes through unrelated root commits.
Description check Passed The description follows the required template and explains the problem, user-visible behavior, implementation, linked issue, visual proof, testing, platform coverage, and checklist status. It is compl…

  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR





  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 1


ℹ️ Review info
⚙️ Run configuration
  • Configuration used: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: b1521a22-60d9-45a9-85ff-6dad36904bda
📥 Commits

Reviewing files that changed from the base of the PR and between 65580c5 and 67f9556.

📒 Files selected for processing (5)
  • src/renderer/src/components/right-sidebar/source-control/sync/git-history-graph-svg.test.tsx
  • src/renderer/src/components/right-sidebar/source-control/sync/git-history-graph-svg.tsx
  • src/shared/git-history-graph.test.ts
  • src/shared/git-history-graph.ts
  • src/shared/git-history-real-repository.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 3 remain after this review.

@nwparker
nwparker merged commit b3aa719 into main Oct 10, 2026
32 checks passed
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.

[Bug]: History graph loses its lines below a second root commit

1 participant