Fixes graph scope mode showing no commits for old branches#5266
Draft
ianhattendorf wants to merge 1 commit into
Draft
Fixes graph scope mode showing no commits for old branches#5266ianhattendorf wants to merge 1 commit into
ianhattendorf wants to merge 1 commit into
Conversation
- Iterates the merge-target candidate chain (stored merge target → base branch → default branch) in `computeScopeAnchor` instead of picking one and bailing when `target === focal`; a branch made via `git branch X origin/Y` has a reflog-derived "base" that's the branch's own upstream, which collapsed to the focal tip and produced a focal-only anchor that left the bare scope rendering 0 rows - Switches `onEnsureRowRequest` to a single streaming `git log --all` call with `limit=0`, engaging `getCommitsForGraphCore`'s "no count cap, break on found" mode; the prior single skip-based page hit the 10× defensive cap (~5000 commits) for shas deep in history, and a naive iterative skip loop would be O(N²) total work for an N-deep target (each `--skip=N` walks N+page commits). One streaming call is O(N) and one git process spawn instead of dozens. - Applies `focalBranchTipSha` onto the scope on the initial-resolve path (not just `patchScopeAnchor`), and applies a partial anchor when only some fields are usable; `_pendingFocalTipBranchRef` in graph-app drains on this field and would otherwise never select an old branch's tip - Stashes the resolved anchor when its merge-base row isn't in the loaded window and re-applies it via the `DidChangeRowsNotification` handler once paging brings the row in; gracefully handles graph-app's parallel `EnsureRowRequest` for the focal tip cancelling ours mid-flight, with `_inFlightEnsureMergeBaseSha` deduping concurrent issues and `_lastEnsureMergeBaseAttempt` preventing retry-storms when the host gives up
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Scoping to an old local branch in the Commit Graph (via the focus picker, sidebar select, or overview-card click) sometimes rendered an empty filtered view: no commits visible despite the scope being active. Three issues at the same fault line, fixed together:
computeScopeAnchorcollapses to a focal-only anchor whentarget === focal. The reflog-derived "base branch" for branches made viagit branch X origin/Yresolves to the branch's own upstream, which equals the focal tip. The original guard returned{focalBranchTipSha}only, so the bare scope never got a real merge base. Fix: iterate the candidate chain (mergeTargetBranch → baseBranch → defaultBranch) and skip candidates that collapse to the focal tip, sodefaultBranch(typicallymain) becomes the recovered merge target.applyAnchorToPendingScopedroppedfocalBranchTipShaon the initial-resolve path. OnlypatchScopeAnchorever set it on the live scope, so_pendingFocalTipBranchRefin graph-app — which drains on this field to load the focal tip — never fired for newly-scoped branches. Fix: applyfocalBranchTipShawhenever the anchor carries one, even if the merge base is unusable.onEnsureRowRequestonly paged once. For shas deep in history (an old branch's tip or its merge base), a singlegit log --skip=N --max-count=500call hit thegetCommitsForGraphCore10× defensive cap before reaching the target. Fix: passlimit=0to engagegetCommitsForGraphCore's existing "no count cap, break on found" mode — one streaminggit log --allinvocation, O(N) walk, single process spawn instead of iteration.Plus the stash + retry machinery (
_pendingAnchorForMergeBaseLoad,tryUpgradeScopeFromPendingAnchor,requestEnsureRowForMergeBase) to handle graph-app's parallelEnsureRowRequestfor the focal tip racing/cancelling our merge-base request — re-issues once when rows arrive without the merge base, gives up cleanly when rows haven't grown across attempts.Verification
Live repro in
~/dev/repos/vscode-gitlensagainst three branches:old-feature-combined-diff(local, 2020-04 tip)old-feature-timeline(local, 2020-12 tip)feature/combined-diff(remote-only)Cold scope-to-deep-branch: ~3.9s to render (single git log walk to the 2020-era merge base, ~9,800 rows fetched). Subsequent scopes that reuse loaded rows: ~250ms.
Known follow-ups (not in this PR)
--shortstatstats follow-up. A targeted slab fetch (git log mergeBase..branchRef) would load ~50–100 commits instead — see review thread for a sketch.getBaseBranchFromRefloginpackages/git-cli/src/providers/branches.tsstill returns the branch's own upstream as "base" forgit branch X origin/Y-style branches. This PR neutralizes the bad value for graph scope, but it still flows to branch-tree comparison defaults, the merge-target picker, and overview enrichment. Worth a separate ticket.Test plan