Record a tree per diff snapshot so the Diff tab shows per-interval changes - #63
Merged
Merged
Conversation
The Diff tab was meant to be a chronological list of unified diffs - what changed in the last five minutes - but nothing recorded a tree per snapshot, so it could only render one cumulative patch and filter it to a snapshot's file list. A file edited at snapshot 1 and again at snapshot 3 showed its state now under either, and a reverted file dropped out entirely behind the notice "changed again since that snapshot". The git engine now writes a tree per snapshot. Staging goes into a per-run store at <checkouts>/<run-id>.diffsnap - a sibling of the checkout, the same sidecar convention the identity record uses - holding an object directory whose alternates file points at the checkout's own database, a persistent index that carries git's stat cache between snapshots, and the last tree id so the interval chain survives a watch or server restart. Nothing under the checkout's .git is written: run provisioning chowned it to the run's user. The store is removed with the checkout, so the trees are reclaimed by the same scheduler GC that already reclaims worktrees and nothing accrues after a run is cleaned up. run.diff carries the snapshot's tree and the tree before it, and run.patch takes from and to to render that one interval instead of the cumulative diff. Client-supplied ids are validated as object ids and resolved only inside the run's own store. The snapshot gate is now the tree with the stat set as fallback, which also closes a hole: an edit that kept line counts identical moved the tree but not the numstat and was dropped silently. The dashboard renders the interval on selection and caches each one - two tree ids address it, so it can never go stale. The filtered view and its notice are gone; a snapshot with no tree is not selectable.
…anges
Review findings on the per-interval diff work.
A persistent index means a persistent index.lock. Git killed at the 30s
snapshot timeout, or a server that died mid-staging, would leave one behind
and every later snapshot for that run would fail with "Unable to create
index.lock: File exists" until checkout GC - per-interval diffs dead, no
error anywhere the member can see. The store is single-writer: one watch
goroutine stages into it, and RemoveRunCheckout stops that watch before
deleting. Any lock found is stale by construction, so staging clears it
first.
The publish gate ORed tree inequality with stat inequality, which the
comment and the commit message both described as tree-with-stats-as-
fallback. It matters after a restart: lastTree is restored from the store
but the stat set starts empty, so the first snapshot of unchanged content
published an interval whose two ends are the same tree. The tree decides
now, and the stats are the fallback only when the tree could not be written.
A range end was resolved with rev-parse id^{tree}, so a commit id from the
history the checkout was cloned with peeled to its tree and rendered a diff
the timeline never offered. Ends must now name a tree object. Existence is
checked with ^{object}, because rev-parse --verify on a bare full-length id
echoes it back without asking the database anything - the all-zero id sailed
through.
Also: the last-tree record is written through a temporary file and renamed,
so a crash leaves the previous id rather than a truncated one; a tree-to-
tree render no longer seeds an index it never writes, which keeps a read
clear of the lock staging takes; and docs/install.md says how the store
grows.
Tests cover a planted stale lock, a watch restart resuming its chain, a
commit id refused as a range end, and the snapshot path leaving the
checkout's .git untouched.
Re-review findings on the per-interval diff. Rendering a range no longer brings a snapshot store into being. A request racing checkout GC would recreate the directory the removal had just deleted, and nothing collects it afterwards; the store is stat'd first and a missing one answers "tree is no longer on disk", which is what it is. The alternates file is now written only when it is missing or wrong, and installed by rename. Every range request used to rewrite it with O_TRUNC while the watch goroutine was doing the same, and a git that opened it in that window saw no alternate at all and reported the run's own trees as gone. On the dashboard, the interval cache was described as bounded by the snapshot list but nothing pruned it: trimming the list now drops the patches its dropped snapshots named. A cached failure is now retryable too - Refresh clears the failed entries, which were otherwise the one thing on the tab no button could recover, while intervals that answered stay, being immutable. listTree digests each file's content, so the assertion that a render leaves the checkout's .git alone can see an in-place rewrite of .git/index rather than only an added or removed path.
not-varram
force-pushed
the
varram/sup-60-record-a-tree-per-diff-snapshot
branch
from
September 4, 2026 03:59
96f177e to
ce49c31
Compare
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
Closes SUP-60. The Diff tab rendered one cumulative patch against the fork point and only filtered it to a snapshot's file list, so selecting a snapshot showed a file's state as of now rather than what that interval changed. Nothing recorded a tree per snapshot, so the per-interval delta was not obtainable.
<data>/checkouts/<run-id>.diffsnap/(anobjects/directory whose alternates file points at the checkout's own database, a persistent index, and alastfile). Nothing under the checkout's.gitis written, since that directory belongs to the run's user. The store is removed with the checkout by the existing GC, and its size lands in the disk gauge'sworktree_bytes.run.diffevents carrytreeandparent_tree; the patch endpoint takes?from=&to=, accepts only full-length hex ids that resolve to tree objects in that run's store, and answersrun.patch: that snapshot's tree is no longer on diskonce the checkout is gone.last.Validation
index.lock(agit addkilled at the 30 s timeout) is cleared and the next snapshot succeeds; restart resumes the interval chain; a commit id and an all-zero id are refused; the checkout's.gitis content-identical after snapshots and renders; a range request after checkout removal does not recreate the store.make fmt-check vet lint test public-audit,go test -race -tags integration ./internal/gitengine/, andmake test-integration(Docker) pass;cd web && bun run typecheck && bun run test(516 tests) pass.O_TRUNCunder a concurrent writer, an unbounded dashboard interval cache, an unretryable cached error). All fixed before opening.🤖 Generated with Claude Code
https://claude.ai/code/session_019irxg6aWWKmn2w3QsJR5CJ