Show branch topology in revision history graph#10
Open
JohnRDOrazio wants to merge 13 commits intomainfrom
Open
Show branch topology in revision history graph#10JohnRDOrazio wants to merge 13 commits intomainfrom
JohnRDOrazio wants to merge 13 commits intomainfrom
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds branch-ref propagation through API, graph builder, and UI; Graph vertices gain refs and lane-hinting; BranchContext switches to React Query, exports branchQueryKeys and refreshBranches; Editor page invalidates branch list queries after save/delete/update; Revision UI and GitGraph render per-commit branch badges. Changes
Sequence Diagram(s)sequenceDiagram
participant Editor as Editor Page
participant Query as React Query Cache
participant BranchProv as BranchContext
participant API as API Layer
Editor->>API: save / update / delete
API-->>Editor: operation result
Editor->>Query: invalidateQueries(branchQueryKeys.list(projectId))
Query->>BranchProv: triggers refetch (refreshBranches)
BranchProv->>API: fetch branches
API-->>BranchProv: branches + defaultBranch
BranchProv-->>Editor: context updates (branches/defaultBranch)
sequenceDiagram
participant UI as RevisionHistoryPanel
participant API as API Layer
participant Builder as Graph Builder
participant Graph as GitGraph Component
participant Context as BranchContext
UI->>API: loadHistory() (commits + refs)
API-->>UI: commits + refs
UI->>Context: read defaultBranch
UI->>Builder: buildGraphLayout(commits, refs, defaultBranch)
Builder->>Builder: computeBranchHints(refs, defaultBranch)
Builder->>Builder: assignLanesAndColors(vertices, hints)
Builder-->>UI: GraphLayout (vertices include refs)
UI->>Graph: render commits with branch badges
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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. Comment |
Member
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@components/revision/RevisionHistoryPanel.tsx`:
- Around line 196-205: The badge strip is wrapping and pushing the text column
out of sync; when rendering refs?.[commit.hash]?.map((ref) => ...) around the
GitBranch badge, make the badge row single-line and truncated by adding no-wrap
and overflow truncation classes to the span (e.g. add whitespace-nowrap,
overflow-hidden, text-ellipsis/truncate and a sensible max-width or max-w-[...]
class) so long branch names don’t wrap; keep the GitBranch icon and ref text but
ensure each badge is constrained (or alternatively constrain the parent
container for all badges) so the row stays fixed-height and aligns with the
graph cell.
In `@lib/context/BranchContext.tsx`:
- Around line 61-70: The BranchProvider currently exposes an imperative
refreshRef (BranchProviderHandle.refreshBranches) and uses loadBranches; replace
this with a React Query-backed query for branches keyed by projectId and remove
refreshRef/refreshBranches from BranchProviderProps/BranchProviderHandle,
switching consumers to the query's data and refetch functions; implement any
write operations as React Query mutations that call
queryClient.invalidateQueries or mutation.onSuccess to refresh the branches
cache, and update callers (the editor and history panel) to either read the
branches from useQuery or call queryClient.refetchQueries/useQueryClient instead
of calling loadBranches or branchRef.current?.refreshBranches().
In `@lib/git-graph/graph-builder.ts`:
- Around line 183-185: The color assignment incorrectly allows the first side
branch to receive color 0 because nextColor is initialized to 0; update the
logic in graph-builder.ts (around the assignedColor/hintedLane handling and the
nextColor initialization) to reserve color 0 for lane 0 — either initialize
nextColor to 1 where it’s declared or ensure the branch that sets assignedColor
for hintedLane !== 0 uses nextColor starting at 1 (and then increments nextColor
afterwards); refer to the variables assignedColor, hintedLane, and nextColor
when making the change.
- Around line 167-171: The forEach callbacks are returning the Set via the
implicit-expression form (triggering useIterableCallbackReturn); change the
callbacks that iterate activeLanes and the other forEach at the later location
to use block bodies and call usedLanes.add(lane); (and the analogous add call)
as statements. Locate the forEach that reads activeLanes and the second forEach
around the code referencing hintedLane and replace the arrow expressions with
block-bodied arrow functions so Set.add(...) is a statement rather than an
implicit return.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 11f84d96-c734-4cba-b909-a989ba5cf677
📒 Files selected for processing (7)
app/projects/[id]/editor/page.tsxcomponents/revision/GitGraph.tsxcomponents/revision/RevisionHistoryPanel.tsxlib/api/revisions.tslib/context/BranchContext.tsxlib/git-graph/graph-builder.tslib/git-graph/types.ts
This was referenced Mar 10, 2026
…nts after commits Use the refs map from the backend to visually separate branches onto different lanes in the git graph. Feature branch commits now appear on a separate lane with branch name badges, while main commits stay on lane 0. Also fix stale branch data (commits ahead/behind) by refreshing branch info after every commit operation and when the history panel opens. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
JohnRDOrazio
force-pushed
the
feature/branch-topology-graph
branch
from
d3fb68f to
3bdf914
Compare
Wrap branch ref badges in a non-wrapping container with overflow hidden to prevent long branch names from breaking row alignment in the git graph. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Use block bodies in forEach callbacks to avoid returning Set.add() values (lint: useIterableCallbackReturn). Initialize nextColor to 1 so the main lane always gets color 0 and side branches start at color 1. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
♻️ Duplicate comments (3)
lib/git-graph/graph-builder.ts (2)
155-156:⚠️ Potential issue | 🟡 MinorKeep color
0exclusive to lane0.
nextColorstill starts at0, so the first hinted side branch on Line 184 renders with the same color as the main lane. The fallback path on Line 216 has the same problem when it allocates lane0.Proposed fix
- let nextColor = 0; + let nextColor = 1; @@ - assignedColor = hintedLane === 0 ? 0 : nextColor++; + assignedColor = hintedLane === 0 ? 0 : nextColor++; @@ - assignedColor = nextColor++; + assignedColor = assignedLane === 0 ? 0 : nextColor++;Also applies to: 183-185, 212-216
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/git-graph/graph-builder.ts` around lines 155 - 156, nextColor is initialized to 0 so the first allocated side branch can get color 0 (same as main lane); change the allocation logic to keep color 0 reserved for lane 0 by initializing nextColor = 1 and update any increment/wrap logic that assigns lanes (places that do nextColor++ / nextColor = (nextColor + 1) % N or fallback lane search) to skip 0 (e.g., if nextColor becomes 0, advance to 1) so no allocation ever returns color 0 for hinted/fallback branches.
167-171:⚠️ Potential issue | 🟠 MajorFix the
forEachcallbacks before linting.Line 171 and Line 210 still use expression-bodied callbacks, so Biome treats them as returning the
SetfromusedLanes.add(...)and failsuseIterableCallbackReturn.Proposed fix
- activeLanes.forEach(({ lane }) => usedLanes.add(lane)); + activeLanes.forEach(({ lane }) => { + usedLanes.add(lane); + }); @@ - activeLanes.forEach(({ lane }) => usedLanes.add(lane)); + activeLanes.forEach(({ lane }) => { + usedLanes.add(lane); + });Also applies to: 209-210
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/git-graph/graph-builder.ts` around lines 167 - 171, The forEach callbacks that call usedLanes.add(...) are using expression-bodied arrows which return the Set value and trigger the useIterableCallbackReturn lint; change those to block-bodied callbacks so they don't return a value. Specifically, update activeLanes.forEach(({ lane }) => usedLanes.add(lane)) to activeLanes.forEach(({ lane }) => { usedLanes.add(lane); }) and make the analogous change to the other forEach near the hintedLane logic (the second occurrence that also calls usedLanes.add) so both callbacks use { ... } and no implicit return.components/revision/RevisionHistoryPanel.tsx (1)
196-205:⚠️ Potential issue | 🟡 MinorConstrain the ref badges to one line.
These rows are fixed to
ROW_HEIGHT, so long or multiple refs can still wrap here and push the text column out of sync with the graph. Keep the badge strip single-line/truncated, or raise both the row height and graph cell height together.Proposed fix
- {refs?.[commit.hash]?.map((ref) => ( - <span - key={ref} - className="inline-flex items-center gap-1 rounded-full bg-blue-100 px-1.5 py-0.5 text-xs font-medium text-blue-700 dark:bg-blue-900/30 dark:text-blue-300" - > - <GitBranch className="h-2.5 w-2.5" /> - {ref} - </span> - ))} + {(refs?.[commit.hash] ?? []).length > 0 && ( + <div className="mt-0.5 flex min-w-0 items-center gap-1 overflow-hidden whitespace-nowrap"> + {(refs?.[commit.hash] ?? []).map((ref) => ( + <span + key={ref} + title={ref} + className="inline-flex max-w-[160px] items-center gap-1 rounded-full bg-blue-100 px-1.5 py-0.5 text-xs font-medium text-blue-700 dark:bg-blue-900/30 dark:text-blue-300" + > + <GitBranch className="h-2.5 w-2.5 shrink-0" /> + <span className="truncate">{ref}</span> + </span> + ))} + </div> + )}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/revision/RevisionHistoryPanel.tsx` around lines 196 - 205, The ref badges can wrap and change row height; wrap the map output in a single-line, non-wrapping container and make each badge non-shrinking so badges are clipped instead of wrapping: update the block that renders refs?.[commit.hash]?.map(...) (the span elements using GitBranch and ref) to be inside a wrapper with CSS classes enforcing overflow-hidden, whitespace-nowrap, and text-overflow/ellipses; ensure each badge gets a flex-shrink-0 (or equivalent) so they stay on one line and excess badges are truncated rather than wrapping.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@components/revision/RevisionHistoryPanel.tsx`:
- Around line 196-205: The ref badges can wrap and change row height; wrap the
map output in a single-line, non-wrapping container and make each badge
non-shrinking so badges are clipped instead of wrapping: update the block that
renders refs?.[commit.hash]?.map(...) (the span elements using GitBranch and
ref) to be inside a wrapper with CSS classes enforcing overflow-hidden,
whitespace-nowrap, and text-overflow/ellipses; ensure each badge gets a
flex-shrink-0 (or equivalent) so they stay on one line and excess badges are
truncated rather than wrapping.
In `@lib/git-graph/graph-builder.ts`:
- Around line 155-156: nextColor is initialized to 0 so the first allocated side
branch can get color 0 (same as main lane); change the allocation logic to keep
color 0 reserved for lane 0 by initializing nextColor = 1 and update any
increment/wrap logic that assigns lanes (places that do nextColor++ / nextColor
= (nextColor + 1) % N or fallback lane search) to skip 0 (e.g., if nextColor
becomes 0, advance to 1) so no allocation ever returns color 0 for
hinted/fallback branches.
- Around line 167-171: The forEach callbacks that call usedLanes.add(...) are
using expression-bodied arrows which return the Set value and trigger the
useIterableCallbackReturn lint; change those to block-bodied callbacks so they
don't return a value. Specifically, update activeLanes.forEach(({ lane }) =>
usedLanes.add(lane)) to activeLanes.forEach(({ lane }) => { usedLanes.add(lane);
}) and make the analogous change to the other forEach near the hintedLane logic
(the second occurrence that also calls usedLanes.add) so both callbacks use {
... } and no implicit return.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 91d32c2b-fe7a-4f33-97dd-d456384614b3
📒 Files selected for processing (7)
app/projects/[id]/editor/page.tsxcomponents/revision/GitGraph.tsxcomponents/revision/RevisionHistoryPanel.tsxlib/api/revisions.tslib/context/BranchContext.tsxlib/git-graph/graph-builder.tslib/git-graph/types.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- lib/api/revisions.ts
- lib/context/BranchContext.tsx
When the fallback lane assignment happens to pick lane 0, use color 0 instead of allocating the next color, keeping main-line commits visually consistent. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
components/revision/RevisionHistoryPanel.tsx (1)
39-71: 🛠️ Refactor suggestion | 🟠 MajorMove the history/branch loading onto React Query.
This change adds more remote state (
refsplus a branch refresh trigger) behind localuseState/useEffect, which keeps cache and refetch behavior split across component state and context. Please model the history request as a query and drive refreshes through query invalidation/refetch instead.As per coding guidelines, "Use React Query (
@tanstack/react-query) for server state management".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/revision/RevisionHistoryPanel.tsx` around lines 39 - 71, The component is managing remote history/refs with local useState and loadHistory; replace that with a React Query useQuery that calls revisionsApi.getHistory and returns {commits, refs} keyed by ['revisions', projectId] (remove loadHistory, setCommits/setRefs, setIsLoading/setError and selected state updates tied to them), enable the query only when isOpen and projectId are present, and surface query status/error instead of local flags; keep calling loadBranches() but when branches refreshes complete (or when defaultBranch changes) trigger the history refresh by calling useQueryClient().invalidateQueries(['revisions', projectId]) or query.refetch so branch updates drive refetches rather than local effects. Ensure you reference the existing symbols revisionsApi.getHistory, loadBranches, loadHistory (remove), commits and refs state (replace), and the query key ['revisions', projectId].
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@lib/git-graph/graph-builder.ts`:
- Around line 130-137: The loop that assigns lane hints (using nextLane, hints,
walk, GraphVertex, vertices, mainLine, and v.id) currently overwrites existing
hints and steals commits from earlier stacked branches; change the walk so it
stops when it encounters a commit that already has a hint instead of rewriting
it (i.e., break the walk if hints.has(walk) is true) — ensure you check hints
before calling hints.set and update the loop condition or add an early-break so
earlier lane assignments are preserved.
---
Outside diff comments:
In `@components/revision/RevisionHistoryPanel.tsx`:
- Around line 39-71: The component is managing remote history/refs with local
useState and loadHistory; replace that with a React Query useQuery that calls
revisionsApi.getHistory and returns {commits, refs} keyed by ['revisions',
projectId] (remove loadHistory, setCommits/setRefs, setIsLoading/setError and
selected state updates tied to them), enable the query only when isOpen and
projectId are present, and surface query status/error instead of local flags;
keep calling loadBranches() but when branches refreshes complete (or when
defaultBranch changes) trigger the history refresh by calling
useQueryClient().invalidateQueries(['revisions', projectId]) or query.refetch so
branch updates drive refetches rather than local effects. Ensure you reference
the existing symbols revisionsApi.getHistory, loadBranches, loadHistory
(remove), commits and refs state (replace), and the query key ['revisions',
projectId].
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: eddff8d5-2c95-49a5-96bb-fa643180da3c
📒 Files selected for processing (2)
components/revision/RevisionHistoryPanel.tsxlib/git-graph/graph-builder.ts
When feature/b branches from feature/a (not main), the second branch walk would overwrite feature/a's lane hints. Add a guard to break early if a commit was already claimed by a prior branch walk. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…reshRef Replace manual useState/useCallback/useEffect branch loading with useQuery. Remove useImperativeHandle/refreshRef pattern — the editor page now invalidates the shared query key directly via useQueryClient. RevisionHistoryPanel no longer needs a manual loadBranches() call. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Both are used in callbacks throughout the file but the build environment's ESLint flags them as unused. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Remove unnecessary eslint-disable comments (values are used in callbacks) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The Dockerfile COPY step expects /app/public to exist. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
lib/context/BranchContext.tsx (1)
82-91: Consider includingaccessTokenin the query key for cache correctness.The query key is
["branches", projectId], butaccessTokenis passed to the query function. IfaccessTokenchanges (e.g., token refresh or user switch) whileprojectIdremains the same, React Query may return stale cached data from the previous token.This is likely acceptable if:
- Token changes always coincide with page refreshes, or
- The accessToken is stable for the session lifetime
If token changes can occur without a refresh, consider adding it to the key:
Potential adjustment
const { data: response, isLoading, error: queryError, } = useQuery({ - queryKey: branchQueryKeys.list(projectId), + queryKey: [...branchQueryKeys.list(projectId), accessToken], queryFn: () => branchesApi.list(projectId, accessToken), enabled: !!projectId, });Note: This would also require updating the invalidation calls in
editor/page.tsxto includeaccessToken.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/context/BranchContext.tsx` around lines 82 - 91, The query key for the useQuery call using branchQueryKeys.list(projectId) must include accessToken to avoid returning cached branch data tied to a different token; update the useQuery invocation (where useQuery is called in BranchContext.tsx and uses branchesApi.list(projectId, accessToken)) to include accessToken in the queryKey (e.g., branchQueryKeys.list(projectId, accessToken)), and then update any places that invalidate or refetch this key (notably the invalidation calls in editor/page.tsx) so they pass the same accessToken when invalidating.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@lib/context/BranchContext.tsx`:
- Around line 82-91: The query key for the useQuery call using
branchQueryKeys.list(projectId) must include accessToken to avoid returning
cached branch data tied to a different token; update the useQuery invocation
(where useQuery is called in BranchContext.tsx and uses
branchesApi.list(projectId, accessToken)) to include accessToken in the queryKey
(e.g., branchQueryKeys.list(projectId, accessToken)), and then update any places
that invalidate or refetch this key (notably the invalidation calls in
editor/page.tsx) so they pass the same accessToken when invalidating.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 84058ed7-92dc-4a83-a2ca-e8ba095fb4ea
📒 Files selected for processing (6)
__tests__/lib/git-graph/graph-builder.test.tsapp/projects/[id]/editor/page.tsxcomponents/revision/RevisionHistoryPanel.tsxlib/context/BranchContext.tsxlib/git-graph/graph-builder.tspublic/.gitkeep
The Dockerfile copies from .next/standalone but next.config.ts was missing output: "standalone", causing Docker builds to fail. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Update branchQueryKeys.list() to accept and include accessToken parameter - Include accessToken in useQuery call to prevent returning cached data tied to different tokens - Update refreshBranches callback to pass accessToken when invalidating - Update all 5 invalidateQueries calls in editor/page.tsx to pass accessToken This ensures branch queries are properly scoped by authentication token, preventing incorrect cache hits when users with different tokens access the same project. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Member
Author
|
Gentle reminder: The commit 'd5f88a9' ("Add standalone output for Docker deployment") appears to be unrelated to the branch topology feature. ✨ For future PRs, consider moving unrelated bugfixes or infrastructure changes to their own dedicated PR or branch. This helps keep:
Example workflow:
Thanks for the contribution! This guideline just helps us maintain clean, focused PRs. 👍 |
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
2 participants
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
refsmap to assign feature branch commits to separate lanes in the git graph, with branch name badges (blue, with GitBranch icon)Test plan
npm run type-checkandnpm run lintpass cleanly🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Refactor
Tests
Closes #8