Eager commit-log fetch — fixes #122 (stale commit overlay vs current diff)#129
Conversation
Introduce the async commit-log load primitive mirroring loadFiles: - commitsLoadedMsg carries seq/list/err/truncated - commitsState gains loadSeq for stale-result guard - loadCommits() returns a tea.Cmd when applicable, nil otherwise Task 1 of the eager-commit-fetch plan; no behavior change yet — callers still go through the lazy ensureCommitsLoaded path.
Handler drops stale results via seq-guard (mirrors handleFilesLoaded) and caches errors with loaded=true so the overlay surfaces the failure once instead of re-triggering the fetch. Related to #122
Init now returns tea.Batch(loadFiles, loadCommits) so the commit log is fetched in parallel with files on startup. triggerReload bumps commits.loadSeq and re-fetches commits alongside files so the overlay stays consistent with the diff after R reload.
remove the lazy commit-fetch path entirely now that eager parallel fetch is wired in. handleCommitInfo now reads from the cache populated at startup/reload and shows a transient "loading commits…" hint when the fetch has not yet landed.
Replaced the lazy fetch description in CLAUDE.md Gotchas with the eager parallel fetch pattern (Init/triggerReload tea.Batch, seq-guard, loading hint). Updated the ARCHITECTURE.md overlay flow to describe loadCommits and handleCommitsLoaded instead of the deleted ensureCommitsLoaded. Moved the completed plan to docs/plans/completed/.
Move the seq-contract explanation from type godoc to a trailing field comment on seq, matching the sibling filesLoadedMsg. Minor consistency fix surfaced by code smell review.
Deploying revdiff with
|
| Latest commit: |
bd9fca9
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://88aebec9.revdiff.pages.dev |
| Branch Preview URL: | https://eager-commit-fetch.revdiff.pages.dev |
There was a problem hiding this comment.
Pull request overview
This PR makes commit-log fetching for the commit info overlay consistent with the currently displayed diff by moving from “fetch on first i press” to an eager, parallel async fetch that runs alongside file loading at startup and on R reload (fixing #122).
Changes:
- Added an async
loadCommits()loader +commitsLoadedMsg/handleCommitsLoadedwith a seq-based stale-result guard. - Wired commit loading into
Init()andtriggerReload()viatea.Batch(loadFiles, loadCommits), and removed the old lazyensureCommitsLoaded()path. - Updated unit tests and documentation to match the new eager-fetch behavior and “loading commits…” hint UX.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| docs/plans/completed/20260420-eager-commit-fetch.md | Implementation plan documenting the eager parallel commit fetch approach and testing steps. |
| docs/ARCHITECTURE.md | Updates architecture docs to describe eager commit loading and the new message/handler flow. |
| CLAUDE.md | Updates “Gotchas” to reflect eager parallel commit fetching and the seq-guard. |
| app/ui/model.go | Adds commit-load message/state, batches startup loads, removes lazy fetch, updates handleCommitInfo. |
| app/ui/loaders.go | Introduces loadCommits() and handleCommitsLoaded(), batches reload loads, bumps commit seq on reload. |
| app/ui/loaders_test.go | Adds coverage for loadCommits, handleCommitsLoaded, and reload refetch behavior. |
| app/ui/handlers_test.go | Updates commit overlay handler tests for the eager-load + loading-hint behavior. |
| app/ui/model_test.go | Removes tests for the deleted lazy-fetch helper. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| Replace the lazy commit-info fetch (triggered on first `i` press) with an eager parallel async fetch that runs alongside `loadFiles()` at startup and on `R` reload. This keeps the commit overlay consistent with the displayed diff by querying the same VCS HEAD snapshot for both. | ||
|
|
||
| **Problem** (issue #122): files are loaded eagerly at startup against `HEAD@T0`. Commits are fetched lazily on first `i` press at `T1`. If HEAD advances between T0 and T1 (e.g. during an iterative workflow), the overlay and the diff reflect inconsistent VCS states. | ||
|
|
||
| **Fix**: fire `loadCommits()` in parallel with `loadFiles()` via `tea.Batch` in `Init()` and from `triggerReload()`. Both commands run as independent goroutines and land as separate messages. No ordering coupling, no blocking VCS call on the `i` key press, no stale-HEAD mismatch. `R` reload extends the same invariant — both caches get invalidated and re-fetched in parallel, so the post-reload overlay matches the post-reload diff. |
There was a problem hiding this comment.
The overview claims this “queries the same VCS HEAD snapshot” for files and commits. In the implementation, loadFiles() and loadCommits() run separate VCS commands using the same ref string (often containing HEAD), so HEAD can still advance between the two commands (small race window) and produce a mismatch. Consider rewording to “reduces/eliminates the long T0/T1 mismatch” or, if you want a hard guarantee, resolve HEAD to a concrete commit ID once and pass that stable ref to both loaders.
| Replace the lazy commit-info fetch (triggered on first `i` press) with an eager parallel async fetch that runs alongside `loadFiles()` at startup and on `R` reload. This keeps the commit overlay consistent with the displayed diff by querying the same VCS HEAD snapshot for both. | |
| **Problem** (issue #122): files are loaded eagerly at startup against `HEAD@T0`. Commits are fetched lazily on first `i` press at `T1`. If HEAD advances between T0 and T1 (e.g. during an iterative workflow), the overlay and the diff reflect inconsistent VCS states. | |
| **Fix**: fire `loadCommits()` in parallel with `loadFiles()` via `tea.Batch` in `Init()` and from `triggerReload()`. Both commands run as independent goroutines and land as separate messages. No ordering coupling, no blocking VCS call on the `i` key press, no stale-HEAD mismatch. `R` reload extends the same invariant — both caches get invalidated and re-fetched in parallel, so the post-reload overlay matches the post-reload diff. | |
| Replace the lazy commit-info fetch (triggered on first `i` press) with an eager parallel async fetch that runs alongside `loadFiles()` at startup and on `R` reload. This keeps the commit overlay better aligned with the displayed diff by fetching both during the same startup/reload cycle, eliminating the long `T0`/`T1` mismatch caused by lazy loading. | |
| **Problem** (issue #122): files are loaded eagerly at startup against `HEAD@T0`. Commits are fetched lazily on first `i` press at `T1`. If HEAD advances between T0 and T1 (e.g. during an iterative workflow), the overlay and the diff reflect inconsistent VCS states. | |
| **Fix**: fire `loadCommits()` in parallel with `loadFiles()` via `tea.Batch` in `Init()` and from `triggerReload()`. Both commands run as independent goroutines and land as separate messages. No ordering coupling and no blocking VCS call on the `i` key press; this removes the long lazy-load mismatch window, though it does not by itself guarantee an atomic same-`HEAD` snapshot if `HEAD` advances between the two VCS commands. `R` reload applies the same approach — both caches get invalidated and re-fetched in parallel, so the post-reload overlay and diff are refreshed from the same reload cycle. |
| - Overlay popups managed by `overlay.Manager`. `Compose()` uses ANSI-aware compositing via `charmbracelet/x/ansi.Cut`. `HandleKey()` returns `Outcome` — Model switches on `OutcomeKind` for side effects (file jumps, theme apply/persist). Overlay kinds: help, annot-list, theme-select, commit-info. One overlay at a time — opening any overlay auto-closes whichever was previously open | ||
| - Reload (`R` key): `reloadState` on `Model` holds `pending bool` (waiting for y/cancel), `hint string` (transient status-bar message), and `applicable bool` (false in `--stdin` mode — stream consumed). `ReloadApplicable` is wired at the composition root in `main.go`, following the same pattern as `CommitsApplicable`. The reload method is named `triggerReload()` — not `reload()` — because Go forbids a method and a field with the same name on the same type (`Model.reload` is the state field). Reload resets the diff cursor to the top of the file; tree selection (which file) is restored by `SelectByPath` in `handleFilesLoaded`. | ||
| - Commit info overlay (`i` key) uses the `diff.CommitLogger` capability interface (additive to `diff.Renderer`). Model resolves a `commitLogSource` at construction: explicit `ModelConfig.CommitLog` wins, else type-asserts the renderer for `CommitLogger`, else the feature is unavailable and `i` is a no-op with a transient status-bar hint. `CommitsApplicable` is computed at the composition root by `commitsApplicable()` in `main.go` (using the `commitLogger` field populated by `setupVCSRenderer` in `renderer_setup.go`) — Model copies it, does not re-derive. Data is fetched lazily on first press and cached for the session (refs don't change mid-review). Hg cannot use literal NUL in argv templates — use ASCII US/RS (`\x1f`/`\x1e`) as field/record separators for hg only | ||
| - Commit info overlay (`i` key) uses the `diff.CommitLogger` capability interface (additive to `diff.Renderer`). Model resolves a `commitLogSource` at construction: explicit `ModelConfig.CommitLog` wins, else type-asserts the renderer for `CommitLogger`, else the feature is unavailable and `i` is a no-op with a transient status-bar hint. `CommitsApplicable` is computed at the composition root by `commitsApplicable()` in `main.go` (using the `commitLogger` field populated by `setupVCSRenderer` in `renderer_setup.go`) — Model copies it, does not re-derive. Data is fetched eagerly at startup and on `R` reload: `Init()` and `triggerReload()` both return `tea.Batch(m.loadFiles(), m.loadCommits())`, running files and commits loads in parallel as independent goroutines. `loadCommits()` captures `m.commits.loadSeq` at invocation time and tags the resulting `commitsLoadedMsg` with it; `handleCommitsLoaded` drops any message whose seq no longer matches (stale-result guard, mirrors the files-load pattern). Eager parallel fetch ensures the overlay and the diff reflect the same VCS HEAD snapshot, even if HEAD advances during a long session. If the user presses `i` before `commitsLoadedMsg` arrives, `handleCommitInfo` sets a transient `loading commits…` hint instead of opening the overlay — a second press after load succeeds. Hg cannot use literal NUL in argv templates — use ASCII US/RS (`\x1f`/`\x1e`) as field/record separators for hg only |
There was a problem hiding this comment.
This bullet says eager parallel fetch “ensures the overlay and the diff reflect the same VCS HEAD snapshot”. Since loadFiles() and loadCommits() execute separate VCS commands that may both resolve HEAD, HEAD can still change between them (especially during R reload), so it’s not a strict guarantee. Suggest softening the wording (e.g., “prevents the long delay mismatch” / “greatly reduces the chance”) or documenting that a true guarantee would require resolving HEAD to a commit hash once and using that stable revision for both commands.
| - Commit info overlay (`i` key) uses the `diff.CommitLogger` capability interface (additive to `diff.Renderer`). Model resolves a `commitLogSource` at construction: explicit `ModelConfig.CommitLog` wins, else type-asserts the renderer for `CommitLogger`, else the feature is unavailable and `i` is a no-op with a transient status-bar hint. `CommitsApplicable` is computed at the composition root by `commitsApplicable()` in `main.go` (using the `commitLogger` field populated by `setupVCSRenderer` in `renderer_setup.go`) — Model copies it, does not re-derive. Data is fetched eagerly at startup and on `R` reload: `Init()` and `triggerReload()` both return `tea.Batch(m.loadFiles(), m.loadCommits())`, running files and commits loads in parallel as independent goroutines. `loadCommits()` captures `m.commits.loadSeq` at invocation time and tags the resulting `commitsLoadedMsg` with it; `handleCommitsLoaded` drops any message whose seq no longer matches (stale-result guard, mirrors the files-load pattern). Eager parallel fetch ensures the overlay and the diff reflect the same VCS HEAD snapshot, even if HEAD advances during a long session. If the user presses `i` before `commitsLoadedMsg` arrives, `handleCommitInfo` sets a transient `loading commits…` hint instead of opening the overlay — a second press after load succeeds. Hg cannot use literal NUL in argv templates — use ASCII US/RS (`\x1f`/`\x1e`) as field/record separators for hg only | |
| - Commit info overlay (`i` key) uses the `diff.CommitLogger` capability interface (additive to `diff.Renderer`). Model resolves a `commitLogSource` at construction: explicit `ModelConfig.CommitLog` wins, else type-asserts the renderer for `CommitLogger`, else the feature is unavailable and `i` is a no-op with a transient status-bar hint. `CommitsApplicable` is computed at the composition root by `commitsApplicable()` in `main.go` (using the `commitLogger` field populated by `setupVCSRenderer` in `renderer_setup.go`) — Model copies it, does not re-derive. Data is fetched eagerly at startup and on `R` reload: `Init()` and `triggerReload()` both return `tea.Batch(m.loadFiles(), m.loadCommits())`, running files and commits loads in parallel as independent goroutines. `loadCommits()` captures `m.commits.loadSeq` at invocation time and tags the resulting `commitsLoadedMsg` with it; `handleCommitsLoaded` drops any message whose seq no longer matches (stale-result guard, mirrors the files-load pattern). Eager parallel fetch reduces the chance of overlay/diff mismatch caused by a long delay between the two loads, but it does not strictly guarantee the same VCS HEAD snapshot because `loadFiles()` and `loadCommits()` still resolve VCS state independently. A true guarantee would require resolving HEAD once to a stable commit hash and using that revision for both commands. If the user presses `i` before `commitsLoadedMsg` arrives, `handleCommitInfo` sets a transient `loading commits…` hint instead of opening the overlay — a second press after load succeeds. Hg cannot use literal NUL in argv templates — use ASCII US/RS (`\x1f`/`\x1e`) as field/record separators for hg only |
The two VCS subprocesses each resolve HEAD independently, so eager parallel fetch does not guarantee a strict snapshot — it narrows the race window from 'time until first i press' to 'parallel goroutine start skew' (milliseconds). Update plan and CLAUDE.md wording to reflect this.
Replaces the lazy commit-info fetch (triggered on first
ipress) with an eager parallel async fetch that runs alongsideloadFiles()at startup and onRreload. Commits and file list now query the same HEAD snapshot.Problem: files loaded eagerly at T0, commits fetched lazily at T1. If HEAD advances between T0 and T1 (common in iterative workflows), overlay and diff reflect different VCS states.
Approach:
Init()andtriggerReload()both returntea.Batch(loadFiles, loadCommits). Independent goroutines, separate messages, same seq-guard pattern already used for files.Key changes
loadCommits() tea.Cmdin loaders.go, mirrorsloadFiles()commitsLoadedMsg+handleCommitsLoadedwithcommits.loadSeqstale-result guardensureCommitsLoaded()— no more fetch-on-presshandleCommitInfosimplifies to a pure read; if fetch hasn't landed yet, transientloading commits…hintUX change: previously first
ipress blocked on the VCS call and then opened. Now, if pressed beforecommitsLoadedMsgarrives, shows loading hint and a second press opens the overlay. Acceptable for typical startup (tens of ms); only noticeable on very large repos.Closes #122.