Commits tab: optional (default OFF, persisted) commit-graph visualization #506

Closed
opened 2026-09-13 23:39:13 +00:00 by crueber · 3 comments
Owner

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)

  • The page is 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 in ParentLinks (l.16) — the graph data is already on the wire.
  • The Commit typedef (web/sdk/src/types.js l.10) already carries sha and parents: 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).
  • Because the commits window is sha-addressed and immutable (SHA_TTL / useResolved, Commits.jsx l.100–128), the derived graph can be memoized per sha+path+skip key alongside the existing useData cache — no new fetch machinery.

Architecture notes

  • Client-side derivation only. Lane assignment is a pure function of the visible commit window: topological order of {sha, parents}, first-parent lane threading, branch-out on multiple parents, merge-in on multiple children. Implement it as a headless pure module in web/src/lib/ (repo convention: unit-testable in Node without DOM, per the settingsNav / diff.js precedents) so node --test web/test/unit/*.test.js covers 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.
  • Toggle persistence: follow the theme precedent in web/src/lib/store.js — a localStorage-backed signal with try/catch around storage access, default OFF when no saved value. Toggle lives in the page's crumbs/toolbar row (Commits.jsx l.144), styled like an existing pill/aria-pressed toggle (see the Watch/Star toggle pattern + shared Icon component in web/src/lib/icons.jsx).
  • Themed lane colors: no color literals in JSX — use CSS variables / Tailwind theme classes that resolve per light/dark mode (the app's two-theme pattern in web/src/ui.css). Lane colors must pass contrast in both themes.
  • Mobile-safe collapse: the graph column adds width to commit-row. At narrow viewports the lane column must collapse gracefully (e.g. hide the rail and keep the existing ParentLinks text, or compress to a single-lane dot indicator) — verify at 390px (document.documentElement.scrollWidth <= clientWidth, no panning). The existing commit-row grid template will need a graph-aware variant.
  • No dependency additions. Law 1 governs: build the lane derivation and SVG/CSS rendering by hand. If the implementer believes a library (e.g. a dag layout package) is genuinely required, that is an explicit law-1 decision for the user, flagged in the PR description before any dependency lands — not assumed.

Acceptance criteria

  • Commits tab renders the list unchanged when the toggle is OFF (the default, fresh visitors included).
  • Toggle state persists across reloads (localStorage, storage-unavailable fallback = OFF), following the store.js theme pattern.
  • With the graph ON: each commit row shows its lane; merge commits show converging lines; branch points show diverging lines; lane colors are themed (no literals) and legible in both light and dark mode.
  • Graph is derived client-side from the existing commits payload {sha, parents} — zero new network requests, zero backend/SDK/wire changes.
  • Lane derivation lives in a headless pure module in web/src/lib/ with node --test unit coverage (linear history, branch, merge, branch-then-merge, empty window).
  • At 390px viewport with the graph ON: no horizontal page overflow (scrollWidth == clientWidth); the collapse behavior is defined and intentional.
  • No npm dependencies added; any law-1 conflict is presented to the user as a flagged decision before merging, never resolved silently.
## 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) - The page is `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 in `ParentLinks` (l.16) — the graph data is already on the wire. - The `Commit` typedef (`web/sdk/src/types.js` l.10) already carries `sha` and `parents: 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). - Because the commits window is sha-addressed and immutable (SHA_TTL / `useResolved`, Commits.jsx l.100–128), the derived graph can be memoized per `sha+path+skip` key alongside the existing `useData` cache — no new fetch machinery. ## Architecture notes - **Client-side derivation only.** Lane assignment is a pure function of the visible commit window: topological order of `{sha, parents}`, first-parent lane threading, branch-out on multiple parents, merge-in on multiple children. Implement it as a headless pure module in `web/src/lib/` (repo convention: unit-testable in Node without DOM, per the `settingsNav` / `diff.js` precedents) so `node --test web/test/unit/*.test.js` covers 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. - **Toggle persistence:** follow the theme precedent in `web/src/lib/store.js` — a `localStorage`-backed signal with try/catch around storage access, default **OFF** when no saved value. Toggle lives in the page's crumbs/toolbar row (`Commits.jsx` l.144), styled like an existing pill/`aria-pressed` toggle (see the Watch/Star toggle pattern + shared `Icon` component in `web/src/lib/icons.jsx`). - **Themed lane colors:** no color literals in JSX — use CSS variables / Tailwind theme classes that resolve per light/dark mode (the app's two-theme pattern in `web/src/ui.css`). Lane colors must pass contrast in both themes. - **Mobile-safe collapse:** the graph column adds width to `commit-row`. At narrow viewports the lane column must collapse gracefully (e.g. hide the rail and keep the existing `ParentLinks` text, or compress to a single-lane dot indicator) — verify at 390px (`document.documentElement.scrollWidth <= clientWidth`, no panning). The existing `commit-row` grid template will need a graph-aware variant. - **No dependency additions.** Law 1 governs: build the lane derivation and SVG/CSS rendering by hand. If the implementer believes a library (e.g. a dag layout package) is genuinely required, that is an explicit **law-1 decision for the user, flagged in the PR description before any dependency lands** — not assumed. ## Acceptance criteria - [ ] Commits tab renders the list unchanged when the toggle is OFF (the default, fresh visitors included). - [ ] Toggle state persists across reloads (localStorage, storage-unavailable fallback = OFF), following the `store.js` theme pattern. - [ ] With the graph ON: each commit row shows its lane; merge commits show converging lines; branch points show diverging lines; lane colors are themed (no literals) and legible in both light and dark mode. - [ ] Graph is derived client-side from the existing `commits` payload `{sha, parents}` — zero new network requests, zero backend/SDK/wire changes. - [ ] Lane derivation lives in a headless pure module in `web/src/lib/` with node --test unit coverage (linear history, branch, merge, branch-then-merge, empty window). - [ ] At 390px viewport with the graph ON: no horizontal page overflow (`scrollWidth == clientWidth`); the collapse behavior is defined and intentional. - [ ] No npm dependencies added; any law-1 conflict is presented to the user as a flagged decision before merging, never resolved silently.
crueber added this to the v1 milestone 2026-09-13 23:39:33 +00:00
Author
Owner

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.

Fix PR: https://git.packden.us/crueber/walhub/pulls/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.
Author
Owner

Review of PR #511 (fix/issue-506, commit 65f2292) — verified in scratch worktree /tmp/pr511 (removed afterward); main worktree untouched and clean.

VERIFIED

  • node --test web/test/unit/*.test.js: 1145 total / 1144 pass / 1 fail — matches the PR description. The 1 failure (smoke.test.js '/setup status', 403 vs 200) is pre-existing and environmental: it fails identically on pristine main (re-ran smoke.test.js on main: same single failure; a stray live server answers :8080 /healthz=200 but /setup=403). Zero PR-caused failures. New commit-graph-506.test.js: 14/14 pass.
  • vite build + esbuild (via web/node_modules/.bin directly; mise pnpm shim unavailable in this shell): both green, rail code present in the hashed bundle.
  • Lane algorithm hand-check (node, no DOM): criss-cross (m2:[a,b], m1:[b,a]) threads m2:0 m1:2 a:0 b:1 root:0 width 3 with converging diagonals on both merges; octopus (3 parents) fans out to width 3 and rejoins; no negative lanes, no duplicate sha in any bottom snapshot, every in-window parent carried or joined. Invariants hold.
  • Toggle: default OFF (readGraphEnabled checks ==='1', try/catch, undefined-localStorage -> false), write never throws; pill + aria-pressed + btn-active in the crumbs row.
  • Memoization: createMemo over h() (sha+path+skip useData window); repo pins exactly one .commits( fetch. hist is the Show-when={h()} alias, so graph().rows[i()] index mapping is exact.
  • Rendering: GraphRail (currentColor svg verticals/diagonals, HTML node dot so tall rows stay round, hollow merge node), class-only .gl-0..7 palette (700-grade light / 400-grade dark, no JSX hex literals), ParentLinks kept.
  • 390px: rail display:none at <=480px with fallback to the 3-column row; 8-lane rail is 100px by arithmetic. Reasoned only — no browser run (no private daemon per workspace rules); noted explicitly.
  • Scope: diff touches only docs/go/12_web_ui.md, web/src/lib/commit-graph.js, web/src/pages/Commits.jsx, web/src/ui.css, web/test/unit/commit-graph-506.test.js. No backend/SDK/wire change, no new deps (runtime dep pin passes), law-12 decision appended.

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).

Review of PR #511 (fix/issue-506, commit 65f2292) — verified in scratch worktree /tmp/pr511 (removed afterward); main worktree untouched and clean. VERIFIED - node --test web/test/unit/*.test.js: 1145 total / 1144 pass / 1 fail — matches the PR description. The 1 failure (smoke.test.js '/setup status', 403 vs 200) is pre-existing and environmental: it fails identically on pristine main (re-ran smoke.test.js on main: same single failure; a stray live server answers :8080 /healthz=200 but /setup=403). Zero PR-caused failures. New commit-graph-506.test.js: 14/14 pass. - vite build + esbuild (via web/node_modules/.bin directly; mise pnpm shim unavailable in this shell): both green, rail code present in the hashed bundle. - Lane algorithm hand-check (node, no DOM): criss-cross (m2:[a,b], m1:[b,a]) threads m2:0 m1:2 a:0 b:1 root:0 width 3 with converging diagonals on both merges; octopus (3 parents) fans out to width 3 and rejoins; no negative lanes, no duplicate sha in any bottom snapshot, every in-window parent carried or joined. Invariants hold. - Toggle: default OFF (readGraphEnabled checks ==='1', try/catch, undefined-localStorage -> false), write never throws; pill + aria-pressed + btn-active in the crumbs row. - Memoization: createMemo over h() (sha+path+skip useData window); repo pins exactly one .commits( fetch. hist is the Show-when={h()} alias, so graph().rows[i()] index mapping is exact. - Rendering: GraphRail (currentColor svg verticals/diagonals, HTML node dot so tall rows stay round, hollow merge node), class-only .gl-0..7 palette (700-grade light / 400-grade dark, no JSX hex literals), ParentLinks kept. - 390px: rail display:none at <=480px with fallback to the 3-column row; 8-lane rail is 100px by arithmetic. Reasoned only — no browser run (no private daemon per workspace rules); noted explicitly. - Scope: diff touches only docs/go/12_web_ui.md, web/src/lib/commit-graph.js, web/src/pages/Commits.jsx, web/src/ui.css, web/test/unit/commit-graph-506.test.js. No backend/SDK/wire change, no new deps (runtime dep pin passes), law-12 decision appended. 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).
Author
Owner

Fixed by PR #511 (review clean — lane algorithm hand-verified incl. criss-cross/octopus, toggle/memo/fetch discipline verified), merged. Closing.

Fixed by PR #511 (review clean — lane algorithm hand-verified incl. criss-cross/octopus, toggle/memo/fetch discipline verified), merged. Closing.
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
crueber/walhub#506
No description provided.