Repository navigation
Watch mode, agent skill, and review-first positioning docs - #1
Merged
Merged
Conversation
Poll the diff scope once a second (git status + numstat + hash-object, all fast subprocesses) and fingerprint the changed-file set — paths, statuses, stats, and content hashes, so an edit that leaves the stat line unchanged still registers. On change, the shown lens refreshes in place: the file lens keeps its selection while reviewed marks reset with the content, and the tests lens re-collects like a staged toggle. The hidden lens is dropped so its next toggle rebuilds against the new state, and a 'watching' tag in the header context shows the session is live. Polling over fsnotify keeps the implementation free of watch-limit and network-filesystem failure modes and adds no dependency. Along the way, lift the diff command's scope-aware file listing into git.ListChanged and route the TUI reload paths through it — which also fixes the editor-return reload in a base-ref diff (rift diff main) collapsing the list to the plain worktree-vs-index set.
rift skill prints an embedded SKILL.md that teaches an AI agent the composable review workflow — --json changeset inspection, structural --print diffs, the --tests lens, and review state via --unreviewed. rift skill path materializes it to ~/.local/share/rift/skills/rift-review/SKILL.md (refreshing it when a newer binary changes the content) and prints the location, so a human can point an agent at rift with one line. Both forms support --json.
README gains a comparison table against the review-first cohort (hunk) and the established tools (lazygit, forgit, delta, difftastic), a watch mode section, and an expanded agent section around rift skill. The design doc and CLAUDE.md record inline agent annotations (file-based, next to the reviewed marks — deliberately no daemon) and mouse support as planned features.
Watch follows the same flag↔key symmetry as the other diff-scope toggles (--staged/s, --tests/t, --unreviewed/U), and the usual moment you want it is after the browser is already open — when an agent starts editing — so restarting with --watch would lose your place. Enabling treats the moment as a change: the tree may have drifted while watch was off, so the fingerprint is seeded from fresh state and the shown lens refreshes against it before the polling chain starts. Ticks and polls carry a watch generation so a chain left in flight by a disable (or a rapid disable/enable) is dropped instead of running a second, parallel polling loop. Offered on working-tree scopes only, matching the flag.
Two answers to review questions the engine previously couldn't give. Renames: committed-range and base-ref listings now run --find-renames, so a moved file reports as one record with both paths and only its real content delta — not a full delete plus add. The old path travels on ChangedFile.OldPath (old_path in --json) into the diff engines: git diffs get both sides in the pathspec, difftastic's direct mode extracts the pre-image from the old path (still written under the new path so both sides share a relative path and the diff header stays clean). The reading legend labels a rename 'old → new', and the tests lens reads a renamed file's old side from its old path, so its specs no longer all report as newly added. Worktree/staged listings intentionally keep git-status semantics — the stage screen operates on real index paths. Moved lines: the e engine toggle becomes a ring — difftastic, git's word-diff view, and a new git-moved view that renders relocated code in git's moved colors (--color-moved=zebra), answering 'moved or rewritten?' at a glance. A separate view rather than a flag because git silently ignores --color-moved under --word-diff; without difftastic installed the ring is the two git views, which also makes the moved view the first engine alternative available in a difftastic-less environment.
Correctness: - ListChanged now gives staged precedence over base, matching the per-file previews, so the s toggle in a base-ref scope lists the same side the previews render (it also absorbs pathspec filtering, which every caller applied by hand). - FilterByPaths matches a rename's old path too, so a file moved out of a pathspec-scoped directory no longer vanishes from the listing. - RawRangeDiffAll/RawWorktreeDiff force --find-renames, keeping the tests lens's added-line index consistent with the listing under diff.renames=false. - The tests-lens spec preview carries the renamed file's old path (Spec.OldPath), so a renamed test file previews as its real delta instead of a whole-file addition contradicting its glyph. - The staged scope's watch fingerprint hashes the index (new StagedBlobHashes) instead of the worktree, so a re-stage with unchanged stats registers and worktree-only edits stop triggering spurious refreshes. - toggleStaged bumps the watch generation and re-arms the chain, so a poll in flight across the flip can't apply the old scope's files. - A failed poll listing is skipped rather than treated as an empty changeset that would blank the lens. - A watch refresh preserves the reader's place: only previews whose record or content hash changed are evicted (SetItemsSelectingStale), and the shown preview isn't re-requested when its file didn't change. - A watch refresh on the tests lens applies fresh specs to the shown lens in place (SpecsChangedMsg), preserving engine/layout/filter/ selection, and cancels a stale lens's streams before replacing it. - A collect dropped by a gen bump is relaunched (watch refresh) or its stale lens discarded (toggle away), so a racing t press isn't lost and stale specs can't persist; dropped StreamReadyMsgs have their cancel invoked so orphaned difftastic runs die. - The engine ring dedupes against the primary engine (a drilldown entered on git-moved no longer holds it twice), omits the moved view without color (its flags are inert), and the stage screen uses a hunk-appropriate ring without the moved view (DiffHunks never engages move detection). CanToggle is meaningful again. - Enabling watch with w keeps the pre-disable fingerprint, so an off/on over an unchanged tree refreshes nothing. Cleanup: - The watching tag lives in tui.ContextLabel; the rename label lives in ChangedFile.DisplayPath; the recollect choreography is one helper; poll hashes ride into applyFiles instead of being recomputed on the event loop; skill.Path derives from tooling.DataDir and writes via temp+rename; SKILL.md's commit-range tests query now names a command that actually exists (rift diff A B --tests --json).
- Engine.Diff takes a diff.File{Path, OldPath} identity instead of a bare
path with OldPath smuggled through the scope-level DiffOpts — a caller
passing shared opts per file can no longer forget the rename pairing.
- The watch fingerprint no longer needs synchronous seeding: watchFP starts
empty and the first poll (startup, scope flip, all off the event loop)
baselines it without refreshing; pollWatch always carries worktree hashes
for the file lens (index hashes are fingerprint-only), deleting the staged
nil-out special case.
- The collecting bool becomes a collectGen stamp (pending iff == gen), so
gen bumps invalidate a pending collect structurally instead of via manual
clears scattered across handlers.
- SetItemsSelecting and SetItemsSelectingStale share one setItems core —
and the stale comparison now strips the cache key's width qualifier,
which the split implementation got wrong (stale entries were never
evicted and a changed selected file could keep serving its old preview).
- The atomic-write pattern gets a shared home (tooling.WriteFileAtomic,
next to DataDir); skill.Path uses it.
Left for follow-ups: routing lensui's remaining concrete-type switches
through the PreviewChild contract, threading a context through
review.Collect so superseded collects can be cancelled, and migrating the
two older atomic-write copies (reviewed.go, difft.go) onto the helper.
One git engine with best-effort defaults (--word-diff=color) instead of two flavors on the e cycle. The moved-line view (--color-moved=zebra) stays on the roadmap as a config option — it can't join the default flags because git silently ignores --color-moved under --word-diff, so the choice between word emphasis and move classification is exactly the kind of one-or-the-other preference config.toml exists for. The exclusivity finding is recorded on displayFlags, in CLAUDE.md's Planned list, and in the design doc so it isn't relearned. Restores the simple difftastic↔git-diff pair (still deduped when a drilldown is seeded with the git engine, still a hidden hint without difftastic) and removes the stage-specific hunk toggle variant that existed only to exclude the moved view.
madhermit
marked this pull request as ready for review
July 13, 2026 22:41
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
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.
Adopts the transferable ideas from a look at hunk, reshaped to fit rift's transient/composable philosophy.
What's here
rift diff --watch— live reload while something else (typically an agent) edits the working tree. Polls the diff scope once a second (status + numstat + hash-object; content-hash fingerprinting so an edit with an unchanged stat line still registers) and refreshes the shown lens in place: the file lens keeps its selection, its cached previews for untouched files, and the reader's scroll position; reviewed marks reset as content changes; the tests lens re-collects and applies specs in place. Works for unstaged/staged (index-hash fingerprinting, so re-stages register) and base-ref scopes; rejected for commit ranges and non-TTY output. Awatchingtag shows in the header, andwtoggles watch live — enabling catches up on drift accumulated while off, and generation counters keep scope flips and rapid toggles from applying stale polls. Deliberately polling, not fsnotify: no watch-limit or NFS failure modes, no new dependency.Rename detection in range/base diffs —
--find-renamesin committed-range and base-ref listings: a moved file reports as oneRrecord with both paths and only its real content delta, not a full delete + add.old_pathrides on the JSON output, pathspec filtering matches either side (a file renamed out of a scoped directory still lists), the reading legend labels the fileold → new, both engines take adiff.File{Path, OldPath}identity so no call site can forget the pairing, and the tests lens reads a renamed file's old side correctly. Worktree/staged listings intentionally keep git-status semantics — the stage screen operates on real index paths.rift skill/rift skill path— an embedded SKILL.md that teaches an agent the composable review workflow (--jsoninspection, structural--printdiffs,--tests,--unreviewed).pathmaterializes it atomically to~/.local/share/rift/skills/rift-review/SKILL.md(refreshed when the binary's copy changes) so a human can say "runrift skill pathand follow that skill".README — a capability comparison table vs hunk, lazygit, forgit, delta, and difftastic, plus watch-mode and agent-skill sections.
Planned-feature docs — inline agent annotations (file-based store next to the reviewed marks,
rift annotate, picked up by--watch; explicitly no daemon, in contrast to hunk's loopback session API), mouse support, and a moved-line diff view as a future config option (git's--color-movedis mutually exclusive with the word-diff emphasis the git view defaults to — the finding is recorded indisplayFlags, CLAUDE.md, and the design doc). An earlier revision shipped the moved view as a thirde-toggle position; it was pulled in favor of one best-effort default, configurable later.Fix (fallout of the refactor) — the TUI's editor-return reload now honors a base-ref scope (
rift diff main+ono longer collapses the list to worktree-vs-index) via the newgit.ListChanged, which also owns the staged-over-base precedence so lists always agree with previews.Review
The branch went through
/code-review --fixat xhigh effort (10 finder angles → adversarial verification → gap sweep): 15 findings, 14 confirmed (several reproduced empirically), all fixed — including watch refreshes resetting the reader's scroll position, renames vanishing from pathspec-scoped listings, a staged-scope fingerprint blind spot, scope-flip races, and engine-ring duplication. A/simplifypass then restructured the plumbing (per-filediff.Fileidentity, baseline-poll seeding off the event loop, a generation-stamped collect state, one shared splitlist core — which surfaced and fixed a cache-key mismatch — and a shared atomic-write helper).Verification
mise run checkgreen throughout (fmt, vet, full suite; new tests for the watch fingerprint, staged blob hashes, staged-over-base precedence, rename records in both-zparsers, an end-to-endDiffBetweenCommitsrename, skill materialization, and thegit diffargument builders).wtoggle on/off/catch-up, staged watch detecting a re-stage with identical stats, base-scopestoggle listing the staged side, renames in the TUI/--json/--print/pathspec-scoped listings, and all fourrift skillinvocations.🤖 Generated with Claude Code
https://claude.ai/code/session_01EB5anNK6czSCdjg51CH7VD