Code tab: per-entry last-commit dates — every tree row currently shows the repo HEAD commit time #301

New issue

Closed

opened 2026-09-10 19:30:26 +00:00 by crueber · 3 comments

crueber commented

2026-09-10 19:30:26 +00:00

Owner

What's wrong

On the Code tab, the right-hand "last modified" column shows the same timestamp for every entry — the repo's last commit — instead of each file/directory's own last-commit time. Screenshot: every file and directory on anon/walhub reads "59 minutes ago", though most entries haven't changed in months.

Root cause (code evidence)

web/src/pages/Tree.jsx:224 — const treeDate = () => latestActivity(getHead()); — a single value (the HEAD commit's date, from the one commits?ref=<tree sha>&n=1 fetch at :217-223) stamped onto every row at :278 (<DateTime value={treeDate()} />). There is no per-entry date anywhere in the row loop.
The API can't supply one today: TreeEntry (internal/api/env.go:325-331) carries only name/type/mode/size/sha, and the tree is built by git ls-tree -z -l (internal/api/bind_wal.go:389-394) — ls-tree has no commit-date data; object SHAs carry no timestamps.

What "correct" means (and what it costs)

GitHub-style per-entry last-commit dates require, per entry, the most recent commit touching that path — a log walk per row (git log -1 --format=%cI -- <path>, or one batched git log --format=... --name-only over the branch fused client/server-side). That is genuinely more expensive than ls-tree; the plan below keeps it honest.

Proposed design

Server: per-entry commit metadata on the tree response. Extend TreeEntry with commit_sha string + commit_time string (RFC 3339, omitted when unknown). Compute with ONE batched git invocation, not N: git log <ref> --format=<marker>%H;%cI --name-status -- <dir> walked just far enough to assign each direct child of the listing its latest touching commit (stop once every entry is assigned — most dirs resolve within a handful of recent commits). Submodule entries (type=commit) get no date. Cap the walk (max-tree-log config, mirroring the [import] bounds pattern) and emit no date for unassigned entries rather than stalling the response.
- Alternative considered: git ls-tree-plus-N log calls — rejected, O(entries) subprocess spawns per page view violates the repo's hot-path cost discipline (law 6 precedent).
Client: render the entry's own date. Tree.jsx drops treeDate()-for-all-rows; each row renders <DateTime value={e.commit_time}> with a muted fallback (em-dash or nothing) when commit_time is absent — never the repo HEAD date as a stand-in. Keep the current single-fetch behavior for the directory header context if wanted, but the per-row column must be per-row truth.
Cache discipline. Tree responses are ref-dependent SWR (sha-keyed ETag per §9.2). Adding commit data derived from the same sha keeps the ETag contract intact — the payload is a pure function of the resolved sha, so no cache-class change.
Scope check: this changes the tree payload shape (TreeEntry gains two optional fields) — additive JSON, but it must go through the route-template twins discipline (law 12) and the SDK's tree type.

Acceptance criteria

Each row on the Code tab shows the most recent commit date that touched that specific file/directory, not the repo's HEAD commit date.
Entries not touched within the capped walk (and submodules) render a neutral fallback, not the HEAD date and not an error.
Per-entry dates are computed with a single batched git invocation per tree listing (bounded by a config cap), not one subprocess per entry.
Empty and degraded repo states unchanged; ETag/SWR behavior on the tree endpoint unchanged (payload remains sha-pure).
Tree payload change goes through all three route twins + SDK type update; headless test for the entry-date assignment logic (walk output → per-entry map, cap behavior included).
Mirrors and normal repos behave identically (mirror at anon/walhub is a good fixture).

## What's wrong On the Code tab, the right-hand "last modified" column shows the **same timestamp for every entry** — the repo's last commit — instead of each file/directory's own last-commit time. Screenshot: every file and directory on `anon/walhub` reads "59 minutes ago", though most entries haven't changed in months. ## Root cause (code evidence) - `web/src/pages/Tree.jsx:224` — `const treeDate = () => latestActivity(getHead());` — a **single value** (the HEAD commit's date, from the one `commits?ref=<tree sha>&n=1` fetch at :217-223) stamped onto **every row** at `:278` (`<DateTime value={treeDate()} />`). There is no per-entry date anywhere in the row loop. - The API can't supply one today: `TreeEntry` (`internal/api/env.go:325-331`) carries only `name/type/mode/size/sha`, and the tree is built by `git ls-tree -z -l` (`internal/api/bind_wal.go:389-394`) — `ls-tree` has no commit-date data; object SHAs carry no timestamps. ## What "correct" means (and what it costs) GitHub-style per-entry last-commit dates require, per entry, the most recent commit touching that path — a log walk per row (`git log -1 --format=%cI -- <path>`, or one batched `git log --format=... --name-only` over the branch fused client/server-side). That is genuinely more expensive than `ls-tree`; the plan below keeps it honest. ## Proposed design 1. **Server: per-entry commit metadata on the tree response.** Extend `TreeEntry` with `commit_sha string` + `commit_time string` (RFC 3339, omitted when unknown). Compute with ONE batched git invocation, not N: `git log <ref> --format=<marker>%H;%cI --name-status -- <dir>` walked just far enough to assign each direct child of the listing its latest touching commit (stop once every entry is assigned — most dirs resolve within a handful of recent commits). Submodule entries (`type=commit`) get no date. Cap the walk (`max-tree-log` config, mirroring the `[import]` bounds pattern) and emit no date for unassigned entries rather than stalling the response. - Alternative considered: `git ls-tree`-plus-N `log` calls — rejected, O(entries) subprocess spawns per page view violates the repo's hot-path cost discipline (law 6 precedent). 2. **Client: render the entry's own date.** `Tree.jsx` drops `treeDate()`-for-all-rows; each row renders `<DateTime value={e.commit_time}>` with a muted fallback (em-dash or nothing) when `commit_time` is absent — never the repo HEAD date as a stand-in. Keep the current single-fetch behavior for the *directory header* context if wanted, but the per-row column must be per-row truth. 3. **Cache discipline.** Tree responses are ref-dependent SWR (sha-keyed ETag per §9.2). Adding commit data derived from the same sha keeps the ETag contract intact — the payload is a pure function of the resolved sha, so no cache-class change. 4. **Scope check:** this changes the tree payload shape (`TreeEntry` gains two optional fields) — additive JSON, but it must go through the route-template twins discipline (law 12) and the SDK's tree type. ## Acceptance criteria - [ ] Each row on the Code tab shows the most recent commit date that touched that specific file/directory, not the repo's HEAD commit date. - [ ] Entries not touched within the capped walk (and submodules) render a neutral fallback, not the HEAD date and not an error. - [ ] Per-entry dates are computed with a single batched git invocation per tree listing (bounded by a config cap), not one subprocess per entry. - [ ] Empty and degraded repo states unchanged; ETag/SWR behavior on the tree endpoint unchanged (payload remains sha-pure). - [ ] Tree payload change goes through all three route twins + SDK type update; headless test for the entry-date assignment logic (walk output → per-entry map, cap behavior included). - [ ] Mirrors and normal repos behave identically (mirror at `anon/walhub` is a good fixture).

crueber referenced this issue from a commit

2026-09-10 21:19:56 +00:00

Fix #301: per-entry last-commit dates on tree listings

crueber referenced this issue from a pull request that will close it,

2026-09-10 21:19:59 +00:00

Fix #301: per-entry last-commit dates #308

crueber commented

2026-09-10 21:20:02 +00:00

Author

Owner

Fixed by #308 (branch fix/issue-301): per-entry commit_sha/commit_time on the tree payload via one batched capped git log walk; UI renders each row's own date with em-dash fallback; HEAD-stamp proxy removed. All suites green; browser proof open (shared-daemon loopback guard).

crueber referenced this issue from a commit

2026-09-10 21:42:06 +00:00

Fix #301 review: positional MARK split — marker-looking paths stay paths

crueber commented

2026-09-10 21:42:20 +00:00

Author

Owner

Review of PR #308 (fix/issue-301, per-entry tree dates) — verified in scratch worktree /tmp/pr308 (since removed), main worktree untouched and clean.

COST VERDICT (law 6 — the load-bearing question): ACCEPTABLE, within budget. Verified tree serving already spawns git ls-tree -l -z per listing (internal/api/bind_wal.go:398); the PR adds exactly ONE git log --max-count=<cap> per listing, skipped for empty/all-submodule listings. Bounded (default 200), ctx-cancellable via exec.CommandContext (timeout kills the walk → fail-open undated, never fails the listing), zero store round trips (local serving copy only), parse early-exits at full coverage. One nit: 04_git.md says the walk “rides the per-repo HTTP semaphore like every other §9 recipe” — verified the JSON-API tree lane does NOT take RepoSemaphores (only smart-HTTP in internal/server/smart.go and SSH do). True only in the weak sense (same treatment as the existing ls-tree/cat-file on that lane). Not blocking.

CORRECTNESS (all verified): newest-wins via first-touch guard; nested paths attribute direct child; subdir prefix handled; submodules dateless; cap→undated (never HEAD stamp); failed walk→undated, listing never fails (TestStampTreeDatesDegraded). ETag=res.SHA still correct — payload is a pure function of immutable history at that sha; cap changes read once in NewEnv and surface via restartKeys, so no stale-ETag window. Proxy removal complete: the commits?n=1 fetch and latestActivity import are gone from web/src/pages/Tree.jsx (latestActivity still live via ActivityStamp.jsx — no dead code). \x00\n-glue fix genuine (integration test runs the real pinned argv). Cap: default 200, validated >=1 fail-closed, setup schema + round-trip test present. argv pinned in docs/go/04_git.md (flag order differs trivially, semantics identical). No new non-stdlib imports. Coverage: internal/api 95.5%, internal/config 95.7% (gate holds). Doc entries in 04/07/11/12 accurate.

ONE REAL BUG FOUND AND FIXED (pushed as e9466c9 to origin/fix/issue-301): internal/api/render.go assignTreeCommitMeta sniffed MARK headers by content in EVERY record position, so a path literally named 'WALHUBTREE ' either poisoned later attributions with a bogus commit (valid-header shape) or cleared haveCommit and orphaned the rest of the commit (near-miss shape like 'WALHUBTREE draft'). Fix: headers split positionally — MARK is a header only in status position (!expectPath); in path position the record is always the path (--no-renames emits no pairs). Added regression subtest 'marker-looking paths stay paths' in tree_dates_test.go — verified it FAILS on the old code and passes on the new.

TESTS (final, on pushed head): go test -race internal/api + internal/config green; internal/server green except TestUIAssetConcepts (missing gitignored web/dist/concepts/push.gif — absent on main too, environmental, unrelated); node --test 586/586 green (scratch worktree initially lacked node_modules/dist — environmental, resolved by linking/copying from main worktree, no main files touched); gofmt + go vet clean. No browser drive per review instructions (noted explicitly: Tree.jsx change is a data-source-only swap into the existing DateTime component; backend table + real-git integration tests pin the behavior).

MERGE RECOMMENDATION: ready to merge (pending CI on the pushed hardening commit e9466c9).

Review of PR #308 (fix/issue-301, per-entry tree dates) — verified in scratch worktree /tmp/pr308 (since removed), main worktree untouched and clean. COST VERDICT (law 6 — the load-bearing question): ACCEPTABLE, within budget. Verified tree serving already spawns `git ls-tree -l -z` per listing (internal/api/bind_wal.go:398); the PR adds exactly ONE `git log --max-count=<cap>` per listing, skipped for empty/all-submodule listings. Bounded (default 200), ctx-cancellable via exec.CommandContext (timeout kills the walk → fail-open undated, never fails the listing), zero store round trips (local serving copy only), parse early-exits at full coverage. One nit: 04_git.md says the walk “rides the per-repo HTTP semaphore like every other §9 recipe” — verified the JSON-API tree lane does NOT take RepoSemaphores (only smart-HTTP in internal/server/smart.go and SSH do). True only in the weak sense (same treatment as the existing ls-tree/cat-file on that lane). Not blocking. CORRECTNESS (all verified): newest-wins via first-touch guard; nested paths attribute direct child; subdir prefix handled; submodules dateless; cap→undated (never HEAD stamp); failed walk→undated, listing never fails (TestStampTreeDatesDegraded). ETag=res.SHA still correct — payload is a pure function of immutable history at that sha; cap changes read once in NewEnv and surface via restartKeys, so no stale-ETag window. Proxy removal complete: the commits?n=1 fetch and latestActivity import are gone from web/src/pages/Tree.jsx (latestActivity still live via ActivityStamp.jsx — no dead code). \\x00\\n-glue fix genuine (integration test runs the real pinned argv). Cap: default 200, validated >=1 fail-closed, setup schema + round-trip test present. argv pinned in docs/go/04_git.md (flag order differs trivially, semantics identical). No new non-stdlib imports. Coverage: internal/api 95.5%, internal/config 95.7% (gate holds). Doc entries in 04/07/11/12 accurate. ONE REAL BUG FOUND AND FIXED (pushed as e9466c9 to origin/fix/issue-301): internal/api/render.go assignTreeCommitMeta sniffed MARK headers by content in EVERY record position, so a path literally named 'WALHUBTREE <sha> <date>' either poisoned later attributions with a bogus commit (valid-header shape) or cleared haveCommit and orphaned the rest of the commit (near-miss shape like 'WALHUBTREE draft'). Fix: headers split positionally — MARK is a header only in status position (!expectPath); in path position the record is always the path (--no-renames emits no pairs). Added regression subtest 'marker-looking paths stay paths' in tree_dates_test.go — verified it FAILS on the old code and passes on the new. TESTS (final, on pushed head): go test -race internal/api + internal/config green; internal/server green except TestUIAssetConcepts (missing gitignored web/dist/concepts/push.gif — absent on main too, environmental, unrelated); node --test 586/586 green (scratch worktree initially lacked node_modules/dist — environmental, resolved by linking/copying from main worktree, no main files touched); gofmt + go vet clean. No browser drive per review instructions (noted explicitly: Tree.jsx change is a data-source-only swap into the existing DateTime component; backend table + real-git integration tests pin the behavior). MERGE RECOMMENDATION: ready to merge (pending CI on the pushed hardening commit e9466c9).

crueber closed this issue

2026-09-10 21:43:49 +00:00

crueber referenced this issue from a commit

2026-09-10 21:43:49 +00:00

Merge pull request 'Fix #301: per-entry last-commit dates' (#308) from fix/issue-301 into main

crueber commented

2026-09-10 21:43:49 +00:00

Author

Owner

Fixed by PR #308 incl. review marker-poisoning fix (batched walk, cost within budget, proxy removed; gates green), merged. Closing.

crueber added the

git storage