Commits tab: optional (default OFF, persisted) commit-graph visualization #506
Labels
No labels
actions
bug
cli
duplicate
enhancement
fork
forum
git storage
help wanted
insights
invalid
issues
moderation
oidc
ownership transfer
packages
pr/merge protection rules
projects
pull requests
question
releases
sponsorships
tags
webhooks
wiki
wontfix
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set
Reference
crueber/walhub#506
Loading…
Reference in a new issue
No description provided.
Delete branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
What's requested
An optional commit-graph visualization on the repo Commits tab — off by default, toggled by the user, and persisted across sessions. When ON, the commit list gains a left-hand lane column showing the DAG: one lane per line of development, branch points where a commit has multiple parents, merge nodes where it has multiple parents into it, and colored lane lines that follow each branch through the visible window.
Evidence / current shape (static read, fresh tree)
web/src/pages/Commits.jsx(213 lines).CommitRow(l.43) renders a 3-column grid (commit-main,commit-sha-col,commit-check); parents are currently shown only as text links inParentLinks(l.16) — the graph data is already on the wire.Committypedef (web/sdk/src/types.jsl.10) already carriesshaandparents: string[]— everything needed to derive lanes client-side is present in the existing commits payload. No backend, SDK, or wire change is required: lanes derive from{sha, parents}ordering within the fetched window (CommitPage, types.js l.22).useResolved, Commits.jsx l.100–128), the derived graph can be memoized persha+path+skipkey alongside the existinguseDatacache — no new fetch machinery.Architecture notes
{sha, parents}, first-parent lane threading, branch-out on multiple parents, merge-in on multiple children. Implement it as a headless pure module inweb/src/lib/(repo convention: unit-testable in Node without DOM, per thesettingsNav/diff.jsprecedents) sonode --test web/test/unit/*.test.jscovers it. Note honestly: pagination (?skip=) windows the graph, so lanes are derived per-window; crossing a page boundary a lane may start mid-history — the toggle default OFF means the user opts into that trade-off, and the pager ("older →") behavior with the graph on should be called out in the implementation.web/src/lib/store.js— alocalStorage-backed signal with try/catch around storage access, default OFF when no saved value. Toggle lives in the page's crumbs/toolbar row (Commits.jsxl.144), styled like an existing pill/aria-pressedtoggle (see the Watch/Star toggle pattern + sharedIconcomponent inweb/src/lib/icons.jsx).web/src/ui.css). Lane colors must pass contrast in both themes.commit-row. At narrow viewports the lane column must collapse gracefully (e.g. hide the rail and keep the existingParentLinkstext, or compress to a single-lane dot indicator) — verify at 390px (document.documentElement.scrollWidth <= clientWidth, no panning). The existingcommit-rowgrid template will need a graph-aware variant.Acceptance criteria
store.jstheme pattern.commitspayload{sha, parents}— zero new network requests, zero backend/SDK/wire changes.web/src/lib/with node --test unit coverage (linear history, branch, merge, branch-then-merge, empty window).scrollWidth == clientWidth); the collapse behavior is defined and intentional.Fix PR: #511 (branch fix/issue-506). Client-only lane derivation, default OFF persisted toggle, zero new requests/deps. Tests: new commit-graph-506.test.js 14/14 green; full suite 1145/1144/1 (1 pre-existing live-server smoke failure, verified on pristine main); vite+esbuild green. Browser proof open (shared-daemon loopback). Do NOT merge — review requested.
Review of PR #511 (fix/issue-506, commit
65f2292) — verified in scratch worktree /tmp/pr511 (removed afterward); main worktree untouched and clean.VERIFIED
FINDINGS: none blocking, no small fixes needed. Two non-blocking nits for the author (optional): (a) criss-cross opens a third lane for the second merge — correct and minimal, just noting the width-3 case renders fine by construction; (b) writeGraphEnabled(false) removes the key instead of writing '0' — consistent with the ==='1' read, pinned by test, fine as-is.
MERGE RECOMMENDATION: ready to merge (review only — not merging per instructions).
Fixed by PR #511 (review clean — lane algorithm hand-verified incl. criss-cross/octopus, toggle/memo/fetch discipline verified), merged. Closing.