mirror of
https://github.com/Twigpine/zero.git
synced 2026-11-10 05:01:28 -10:00
fix(tui): keep file-view revisions monotonic and revalidate on git sweeps
Shared path revisions reset to zero on every cache purge, while each surface
kept its own required revision. Three sites could also raise that revision
past the shared one by incrementing it locally. After a purge, an exact-path
edit made on the other BTW surface could produce a shared revision no higher
than this surface's own. recoverInvalidatedFileView then saw nothing new, and
the view stayed stale. Revisions now come from one cache-wide clock that
survives purges. A purge raises the epoch floor to that clock instead of
resetting it, and each surface adopts the revision the cache returns.
Every git sweep invalidated each swept path and forced a full refresh. The
swept list is cumulative since the baseline, and each live sweep is launched
beside the tool-result, turn-end or BTW-return handling that already reloads
or rechecks the open view. The sweep that followed therefore reread and
re-highlighted an unchanged file and flashed "Loading…". A sweep now
revalidates the open view instead. It waits for an in-flight load to land,
then rereads the bounded source in the background and compares it with the
displayed snapshot's identity (hash and truncation flags). The view reloads
only if the bytes changed. A mutation the swept list omits is still caught.
Regression tests (all fail on 311290c0 and pass here):
- TestFileViewCrossSurfaceExactEditAfterPurgeReloadsOtherSurface
- TestFileViewGitSweepRevalidatesWithoutDuplicateReload
- TestFileViewGitSweepDuringLoadDefersToThatLoad
- TestFileViewStaleRevalidationDoesNotReloadNewerSnapshot
- TestFileViewCachePurgeResetsPathRevisions (now asserts monotonicity)
Co-Authored-By: Claude Opus 5.5 <[email protected]>
This commit is contained in:
5 files changed
+447
-78
No files matched your search
@@ -314,6 +314,13 @@ func (m model) routeBTWParentMessage(msg tea.Msg) (model, tea.Cmd, bool) {
|
||||
}
|
||||
return m, nil, false
|
||||
}
|
||||
if revalidated, ok := msg.(fileViewRevalidatedMsg); ok {
|
||||
// A revalidation the parent started before /btw is routed the same way.
|
||||
if m.btw.parent.fileView.active && m.btw.parent.fileView.lifetimeToken == revalidated.lifetimeToken {
|
||||
return m.routeBTWMessageToParent(msg)
|
||||
}
|
||||
return m, nil, false
|
||||
}
|
||||
runID, ok := btwMessageRunID(msg)
|
||||
if !ok || runID <= 0 || runID >= m.btw.sideRunIDBase {
|
||||
return m, nil, false
|
||||
|
||||
+166
-50
@@ -134,6 +134,19 @@ func (e *fileViewCachedEntry) byteSize() int {
|
||||
return n
|
||||
}
|
||||
|
||||
// fileViewSourceIdentity is what decides whether two bounded reads show the
|
||||
// same file: the retained source hash plus both truncation flags.
|
||||
type fileViewSourceIdentity struct {
|
||||
hash [sha256.Size]byte
|
||||
truncated bool
|
||||
omittedLines bool
|
||||
}
|
||||
|
||||
// fileViewReadIdentity returns the identity of one bounded read.
|
||||
func fileViewReadIdentity(res fileViewReadResult) fileViewSourceIdentity {
|
||||
return fileViewSourceIdentity{hash: fileViewSourceHash(res.lines), truncated: res.truncated, omittedLines: res.omittedLines}
|
||||
}
|
||||
|
||||
// fileViewSourceHash fingerprints the bounded source lines so a metadata-only
|
||||
// change (cp -p, rsync -t, tar -xp restoring mtime and size) cannot serve a
|
||||
// stale entry. Lines are joined with a separator the reader never emits inside
|
||||
@@ -215,7 +228,12 @@ type fileViewRenderCache struct {
|
||||
revOrder *list.List // LRU of revision keys: front = most recently invalidated path
|
||||
revElems map[string]*list.Element // targetPath -> *list.Element containing the path string
|
||||
revEpochFloor uint64 // minimum required revision for any path absent from pathRevisions
|
||||
statsData fileViewCacheStats
|
||||
// revClock is the last revision issued to any path. It never decreases,
|
||||
// including across purges, so every new invalidation is newer than any
|
||||
// revision a surface already holds and a path's required revision never
|
||||
// moves backwards.
|
||||
revClock uint64
|
||||
statsData fileViewCacheStats
|
||||
}
|
||||
|
||||
var defaultFileViewCache = newFileViewRenderCache(defaultFileViewCacheMaxEntries, defaultFileViewCacheMaxBytes)
|
||||
@@ -232,14 +250,15 @@ func newFileViewRenderCache(maxEntries int, maxBytes int) *fileViewRenderCache {
|
||||
}
|
||||
}
|
||||
|
||||
// invalidatePath marks targetPath as mutated and returns its new required
|
||||
// revision. Revisions come from one cache-wide clock, so the result is newer
|
||||
// than every revision issued before it, including ones a BTW surface kept from
|
||||
// before a purge. Surfaces adopt this value rather than counting on their own.
|
||||
func (c *fileViewRenderCache) invalidatePath(targetPath string) uint64 {
|
||||
c.mu.Lock()
|
||||
defer c.mu.Unlock()
|
||||
rev := c.pathRevisions[targetPath]
|
||||
if rev < c.revEpochFloor {
|
||||
rev = c.revEpochFloor
|
||||
}
|
||||
rev++
|
||||
c.revClock++
|
||||
rev := c.revClock
|
||||
c.pathRevisions[targetPath] = rev
|
||||
c.touchRevisionLocked(targetPath)
|
||||
c.trimRevisionsLocked()
|
||||
@@ -379,6 +398,11 @@ func (c *fileViewRenderCache) evictPath(targetPath string) {
|
||||
c.mu.Unlock()
|
||||
}
|
||||
|
||||
// purgeLocked drops every entry and per-path revision key. The epoch floor
|
||||
// moves up to the revision clock instead of resetting, so no path's required
|
||||
// revision falls below a revision it already reported. Resetting it would let a
|
||||
// surface hold a revision newer than the shared one, and that surface would
|
||||
// miss a later exact-path edit made on the other surface.
|
||||
func (c *fileViewRenderCache) purgeLocked() {
|
||||
c.gen++
|
||||
c.items = make(map[string]*list.Element)
|
||||
@@ -387,7 +411,7 @@ func (c *fileViewRenderCache) purgeLocked() {
|
||||
c.pathRevisions = make(map[string]uint64)
|
||||
c.revOrder = list.New()
|
||||
c.revElems = make(map[string]*list.Element)
|
||||
c.revEpochFloor = 0
|
||||
c.revEpochFloor = c.revClock
|
||||
}
|
||||
|
||||
func (c *fileViewRenderCache) clear() {
|
||||
@@ -722,28 +746,28 @@ func fileViewSuperseded(liveSeq *atomic.Uint64, seq uint64) bool {
|
||||
return liveSeq != nil && liveSeq.Load() != seq
|
||||
}
|
||||
|
||||
func (c *fileViewRenderCache) loadAndRender(targetPath string, displayPath string, width int, changed map[string]bool, changedFingerprint string, reqGen int, theme tuiTheme, reqSourceRev uint64, liveSeq *atomic.Uint64, seq uint64) (string, error) {
|
||||
func (c *fileViewRenderCache) loadAndRenderSource(targetPath string, displayPath string, width int, changed map[string]bool, changedFingerprint string, reqGen int, theme tuiTheme, reqSourceRev uint64, liveSeq *atomic.Uint64, seq uint64) (string, fileViewSourceIdentity, error) {
|
||||
if fileViewSuperseded(liveSeq, seq) {
|
||||
return "", errFileViewSuperseded
|
||||
return "", fileViewSourceIdentity{}, errFileViewSuperseded
|
||||
}
|
||||
if fileViewInsideLoad != nil {
|
||||
fileViewInsideLoad()
|
||||
}
|
||||
if fileViewSuperseded(liveSeq, seq) {
|
||||
return "", errFileViewSuperseded
|
||||
return "", fileViewSourceIdentity{}, errFileViewSuperseded
|
||||
}
|
||||
|
||||
stat, err := os.Stat(targetPath)
|
||||
if err != nil {
|
||||
c.evictPath(targetPath)
|
||||
rendered := theme.faint.Render("Could not read file: " + err.Error())
|
||||
return rendered, err
|
||||
return rendered, fileViewSourceIdentity{}, err
|
||||
}
|
||||
if !stat.Mode().IsRegular() {
|
||||
c.evictPath(targetPath)
|
||||
readErr := fmt.Errorf("cannot display non-regular file %s (mode %s)", displayPath, stat.Mode().String())
|
||||
rendered := theme.faint.Render("Could not read file: " + readErr.Error())
|
||||
return rendered, readErr
|
||||
return rendered, fileViewSourceIdentity{}, readErr
|
||||
}
|
||||
|
||||
modTime := stat.ModTime()
|
||||
@@ -753,7 +777,7 @@ func (c *fileViewRenderCache) loadAndRender(targetPath string, displayPath strin
|
||||
c.mu.Lock()
|
||||
if c.gen != reqGen {
|
||||
c.mu.Unlock()
|
||||
return "", errors.New("request superseded by cache invalidation")
|
||||
return "", fileViewSourceIdentity{}, errors.New("request superseded by cache invalidation")
|
||||
}
|
||||
c.statsData.DiskReads++
|
||||
c.mu.Unlock()
|
||||
@@ -762,21 +786,22 @@ func (c *fileViewRenderCache) loadAndRender(targetPath string, displayPath strin
|
||||
fileViewBeforeDiskRead()
|
||||
}
|
||||
if fileViewSuperseded(liveSeq, seq) {
|
||||
return "", errFileViewSuperseded
|
||||
return "", fileViewSourceIdentity{}, errFileViewSuperseded
|
||||
}
|
||||
|
||||
readRes := readFileViewBoundedCancellable(targetPath, fileViewMaxLines, fileViewMaxLineBytes, fileViewMaxBytes, liveSeq, seq)
|
||||
if readRes.err != nil && len(readRes.lines) == 0 {
|
||||
c.evictPath(targetPath)
|
||||
rendered := theme.faint.Render("Could not read file: " + readRes.err.Error())
|
||||
return rendered, readRes.err
|
||||
return rendered, fileViewSourceIdentity{}, readRes.err
|
||||
}
|
||||
sourceHash := fileViewSourceHash(readRes.lines)
|
||||
identity := fileViewReadIdentity(readRes)
|
||||
sourceHash := identity.hash
|
||||
|
||||
c.mu.Lock()
|
||||
if c.gen != reqGen {
|
||||
c.mu.Unlock()
|
||||
return "", errors.New("request superseded by cache invalidation")
|
||||
return "", identity, errors.New("request superseded by cache invalidation")
|
||||
}
|
||||
|
||||
curRequiredRev := c.requiredRevisionLocked(targetPath)
|
||||
@@ -796,21 +821,21 @@ func (c *fileViewRenderCache) loadAndRender(targetPath string, displayPath strin
|
||||
entry.omittedLines == readRes.omittedLines {
|
||||
if fileViewSuperseded(liveSeq, seq) {
|
||||
c.mu.Unlock()
|
||||
return "", errFileViewSuperseded
|
||||
return "", identity, errFileViewSuperseded
|
||||
}
|
||||
c.statsData.CacheHits++
|
||||
c.lru.MoveToFront(elem)
|
||||
c.mu.Unlock()
|
||||
|
||||
if rendered, ok := entry.getRender(renderKey); ok {
|
||||
return rendered, nil
|
||||
return rendered, identity, nil
|
||||
}
|
||||
|
||||
if fileViewBeforeCacheHitFormat != nil {
|
||||
fileViewBeforeCacheHitFormat()
|
||||
}
|
||||
if fileViewSuperseded(liveSeq, seq) {
|
||||
return "", errFileViewSuperseded
|
||||
return "", identity, errFileViewSuperseded
|
||||
}
|
||||
|
||||
// Re-format for the new width or changed markers using cached display and lines
|
||||
@@ -819,16 +844,16 @@ func (c *fileViewRenderCache) loadAndRender(targetPath string, displayPath strin
|
||||
fileViewBeforeCacheCommit()
|
||||
}
|
||||
if fileViewSuperseded(liveSeq, seq) {
|
||||
return "", errFileViewSuperseded
|
||||
return "", identity, errFileViewSuperseded
|
||||
}
|
||||
c.commitRenderPut(entry, renderKey, rendered)
|
||||
return rendered, nil
|
||||
return rendered, identity, nil
|
||||
}
|
||||
}
|
||||
|
||||
if fileViewSuperseded(liveSeq, seq) {
|
||||
c.mu.Unlock()
|
||||
return "", errFileViewSuperseded
|
||||
return "", identity, errFileViewSuperseded
|
||||
}
|
||||
|
||||
c.statsData.CacheMisses++
|
||||
@@ -843,7 +868,7 @@ func (c *fileViewRenderCache) loadAndRender(targetPath string, displayPath strin
|
||||
fileViewBeforeHighlight()
|
||||
}
|
||||
if fileViewSuperseded(liveSeq, seq) {
|
||||
return "", errFileViewSuperseded
|
||||
return "", identity, errFileViewSuperseded
|
||||
}
|
||||
|
||||
display, ok := func() ([]string, bool) {
|
||||
@@ -864,7 +889,7 @@ func (c *fileViewRenderCache) loadAndRender(targetPath string, displayPath strin
|
||||
return highlightCodeForPathWithTheme(cleanLines, displayPath, 1<<20, nil, theme)
|
||||
}()
|
||||
if fileViewSuperseded(liveSeq, seq) {
|
||||
return "", errFileViewSuperseded
|
||||
return "", identity, errFileViewSuperseded
|
||||
}
|
||||
if !ok || len(display) != len(cleanLines) {
|
||||
display = cleanLines
|
||||
@@ -874,7 +899,7 @@ func (c *fileViewRenderCache) loadAndRender(targetPath string, displayPath strin
|
||||
fileViewBeforeFormat()
|
||||
}
|
||||
if fileViewSuperseded(liveSeq, seq) {
|
||||
return "", errFileViewSuperseded
|
||||
return "", identity, errFileViewSuperseded
|
||||
}
|
||||
|
||||
rendered := formatFileViewLines(cleanLines, display, changed, readRes.truncated, readRes.omittedLines, width, theme)
|
||||
@@ -883,7 +908,7 @@ func (c *fileViewRenderCache) loadAndRender(targetPath string, displayPath strin
|
||||
fileViewBeforeCacheCommit()
|
||||
}
|
||||
if fileViewSuperseded(liveSeq, seq) {
|
||||
return "", errFileViewSuperseded
|
||||
return "", identity, errFileViewSuperseded
|
||||
}
|
||||
|
||||
sourceBytes := 0
|
||||
@@ -913,24 +938,24 @@ func (c *fileViewRenderCache) loadAndRender(targetPath string, displayPath strin
|
||||
c.mu.Lock()
|
||||
if c.gen != reqGen {
|
||||
c.mu.Unlock()
|
||||
return "", errors.New("request superseded by cache invalidation")
|
||||
return "", identity, errors.New("request superseded by cache invalidation")
|
||||
}
|
||||
if fileViewSuperseded(liveSeq, seq) {
|
||||
c.mu.Unlock()
|
||||
return "", errFileViewSuperseded
|
||||
return "", identity, errFileViewSuperseded
|
||||
}
|
||||
if c.maxEntries <= 0 || c.maxBytes <= 0 || entry.byteSize() > c.maxBytes {
|
||||
c.statsData.SkippedOversized++
|
||||
c.removeItemLocked(targetPath)
|
||||
c.mu.Unlock()
|
||||
return rendered, nil
|
||||
return rendered, identity, nil
|
||||
}
|
||||
|
||||
if elem, ok := c.items[targetPath]; ok {
|
||||
existing := elem.Value.(*fileViewCachedEntry)
|
||||
if existing.modTime.After(readRes.modTime) && existing.sourceRev > reqSourceRev {
|
||||
c.mu.Unlock()
|
||||
return rendered, nil
|
||||
return rendered, identity, nil
|
||||
}
|
||||
c.removeItemLocked(targetPath)
|
||||
}
|
||||
@@ -939,7 +964,13 @@ func (c *fileViewRenderCache) loadAndRender(targetPath string, displayPath strin
|
||||
c.evictOverflowLocked()
|
||||
c.mu.Unlock()
|
||||
|
||||
return rendered, nil
|
||||
return rendered, identity, nil
|
||||
}
|
||||
|
||||
// loadAndRender is loadAndRenderSource without the source identity.
|
||||
func (c *fileViewRenderCache) loadAndRender(targetPath string, displayPath string, width int, changed map[string]bool, changedFingerprint string, reqGen int, theme tuiTheme, reqSourceRev uint64, liveSeq *atomic.Uint64, seq uint64) (string, error) {
|
||||
rendered, _, err := c.loadAndRenderSource(targetPath, displayPath, width, changed, changedFingerprint, reqGen, theme, reqSourceRev, liveSeq, seq)
|
||||
return rendered, err
|
||||
}
|
||||
|
||||
// sanitizeRawFileLine strips or transforms raw control characters and terminal escape sequences
|
||||
@@ -979,7 +1010,9 @@ type fileViewLoadedMsg struct {
|
||||
width int
|
||||
fingerprint string
|
||||
rendered string
|
||||
err error
|
||||
// source identifies the bytes behind rendered; zero when err is set.
|
||||
source fileViewSourceIdentity
|
||||
err error
|
||||
}
|
||||
|
||||
func loadFileViewCmd(targetPath string, displayPath string, width int, changed map[string]bool, fingerprint string, token [16]byte, seq uint64, gen int, reqRev uint64, theme tuiTheme, liveSeq *atomic.Uint64) tea.Cmd {
|
||||
@@ -987,7 +1020,7 @@ func loadFileViewCmd(targetPath string, displayPath string, width int, changed m
|
||||
if fileViewSuperseded(liveSeq, seq) {
|
||||
return fileViewLoadedMsg{lifetimeToken: token, seq: seq, requiredSourceRev: reqRev, generation: gen, targetPath: targetPath, displayPath: displayPath, width: width, fingerprint: fingerprint, err: errFileViewSuperseded}
|
||||
}
|
||||
rendered, err := defaultFileViewCache.loadAndRender(targetPath, displayPath, width, changed, fingerprint, gen, theme, reqRev, liveSeq, seq)
|
||||
rendered, source, err := defaultFileViewCache.loadAndRenderSource(targetPath, displayPath, width, changed, fingerprint, gen, theme, reqRev, liveSeq, seq)
|
||||
return fileViewLoadedMsg{
|
||||
lifetimeToken: token,
|
||||
seq: seq,
|
||||
@@ -998,11 +1031,40 @@ func loadFileViewCmd(targetPath string, displayPath string, width int, changed m
|
||||
width: width,
|
||||
fingerprint: fingerprint,
|
||||
rendered: rendered,
|
||||
source: source,
|
||||
err: err,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// fileViewRevalidatedMsg reports whether the open full-file view's source still
|
||||
// matches the snapshot it is showing. seq is the loadedSeq of that snapshot.
|
||||
type fileViewRevalidatedMsg struct {
|
||||
lifetimeToken [16]byte
|
||||
displayPath string
|
||||
seq uint64
|
||||
changed bool
|
||||
}
|
||||
|
||||
// revalidateFileViewCmd rereads the bounded source off the update goroutine and
|
||||
// compares it with the displayed snapshot's identity. It neither highlights
|
||||
// nor touches the cache. A read cancelled by a newer load reports no change,
|
||||
// because that load reads the file itself.
|
||||
func revalidateFileViewCmd(targetPath string, displayPath string, token [16]byte, shown fileViewSourceIdentity, liveSeq *atomic.Uint64, seq uint64) tea.Cmd {
|
||||
return func() tea.Msg {
|
||||
res := readFileViewBoundedCancellable(targetPath, fileViewMaxLines, fileViewMaxLineBytes, fileViewMaxBytes, liveSeq, seq)
|
||||
changed := false
|
||||
switch {
|
||||
case errors.Is(res.err, errFileViewSuperseded):
|
||||
case res.err != nil && len(res.lines) == 0:
|
||||
changed = true
|
||||
default:
|
||||
changed = fileViewReadIdentity(res) != shown
|
||||
}
|
||||
return fileViewRevalidatedMsg{lifetimeToken: token, displayPath: displayPath, seq: seq, changed: changed}
|
||||
}
|
||||
}
|
||||
|
||||
// fileViewState manages the drill-in view for a touched file. When active, the
|
||||
// transcript body swaps to the file's diff/content instead of the chat rows.
|
||||
type fileViewState struct {
|
||||
@@ -1038,10 +1100,14 @@ type fileViewState struct {
|
||||
loadedToken [16]byte
|
||||
loadedSeq uint64
|
||||
loadedRev uint64
|
||||
loading bool
|
||||
hasError bool
|
||||
snapshotReady bool
|
||||
liveSeq *atomic.Uint64
|
||||
loadedSource fileViewSourceIdentity
|
||||
// revalidateOnLoad defers a git-sweep revalidation until the in-flight load
|
||||
// lands, so the sweep neither repeats nor cancels that load's work.
|
||||
revalidateOnLoad bool
|
||||
loading bool
|
||||
hasError bool
|
||||
snapshotReady bool
|
||||
liveSeq *atomic.Uint64
|
||||
}
|
||||
|
||||
func (m model) startFileViewLoadCmd(width int) (model, tea.Cmd) {
|
||||
@@ -1071,12 +1137,7 @@ func (m model) startFileViewLoad(width int, refreshSource bool) (model, tea.Cmd)
|
||||
target = filepath.Join(m.cwd, target)
|
||||
}
|
||||
if refreshSource {
|
||||
rev := defaultFileViewCache.invalidatePath(target)
|
||||
if rev > m.fileView.requiredSourceRev {
|
||||
m.fileView.requiredSourceRev = rev
|
||||
} else {
|
||||
m.fileView.requiredSourceRev++
|
||||
}
|
||||
m.fileView.requiredSourceRev = defaultFileViewCache.invalidatePath(target)
|
||||
}
|
||||
curReq := defaultFileViewCache.requiredRevision(target)
|
||||
if curReq > m.fileView.requiredSourceRev {
|
||||
@@ -1142,6 +1203,8 @@ func (m model) openFileView(path string) (model, tea.Cmd) {
|
||||
m.fileView.loadedToken = [16]byte{}
|
||||
m.fileView.loadedSeq = 0
|
||||
m.fileView.loadedRev = 0
|
||||
m.fileView.loadedSource = fileViewSourceIdentity{}
|
||||
m.fileView.revalidateOnLoad = false
|
||||
m.fileView.requiredSourceRev = defaultFileViewCache.requiredRevision(target)
|
||||
m.fileView.hasError = false
|
||||
// A file only the git sweep knows about (bash/subagent mutation) has no edit
|
||||
@@ -1225,6 +1288,7 @@ func (m model) handleFileViewLoaded(msg fileViewLoadedMsg) (model, tea.Cmd) {
|
||||
m.fileView.loadedToken = msg.lifetimeToken
|
||||
m.fileView.loadedSeq = msg.seq
|
||||
m.fileView.loadedRev = msg.requiredSourceRev
|
||||
m.fileView.loadedSource = msg.source
|
||||
m.fileView.hasError = (msg.err != nil)
|
||||
// Reconcile the held reading offset against the real body now that the
|
||||
// loading placeholder is gone. Apply the height delta exactly once to
|
||||
@@ -1239,9 +1303,66 @@ func (m model) handleFileViewLoaded(msg fileViewLoadedMsg) (model, tea.Cmd) {
|
||||
}
|
||||
}
|
||||
m.fileView.completedBodyLines = current
|
||||
if m.fileView.revalidateOnLoad {
|
||||
m.fileView.revalidateOnLoad = false
|
||||
return m.revalidateFileView()
|
||||
}
|
||||
return m, nil
|
||||
}
|
||||
|
||||
// fileViewSnapshotCurrent reports whether the completed snapshot is still the
|
||||
// authoritative body for the open view.
|
||||
func (m model) fileViewSnapshotCurrent() bool {
|
||||
return m.fileView.snapshotReady &&
|
||||
m.fileView.loadedPath == m.fileView.path &&
|
||||
m.fileView.loadedSeq == m.fileView.desiredSeq &&
|
||||
m.fileView.loadedRev >= m.fileView.requiredSourceRev &&
|
||||
m.fileView.loadedGen == defaultFileViewCache.generation() &&
|
||||
m.fileView.loadedToken == m.fileView.lifetimeToken
|
||||
}
|
||||
|
||||
// revalidateFileView handles a git sweep for the open full-file view. A sweep
|
||||
// lists every path dirty since the session baseline, not what changed since
|
||||
// this snapshot, and it lands after the tool-result, turn-end, or BTW-return
|
||||
// handling it started beside. So it checks the file content: a current
|
||||
// snapshot stays on screen while the source is reread and compared, and the
|
||||
// view reloads only if the bytes differ. An in-flight load is left to finish
|
||||
// and then checked. A snapshot that is not current, including a read error,
|
||||
// takes an ordinary load.
|
||||
func (m model) revalidateFileView() (model, tea.Cmd) {
|
||||
if !m.fileView.active || m.fileView.mode != fileViewFull || m.fileView.path == "" {
|
||||
return m, nil
|
||||
}
|
||||
if m.fileView.loading {
|
||||
m.fileView.revalidateOnLoad = true
|
||||
return m, nil
|
||||
}
|
||||
if !m.fileViewSnapshotCurrent() || m.fileView.hasError || m.fileView.liveSeq == nil {
|
||||
return m.startFileViewLoadCmd(m.chatColumnWidth())
|
||||
}
|
||||
target := m.fileView.path
|
||||
if !filepath.IsAbs(target) {
|
||||
target = filepath.Join(m.cwd, target)
|
||||
}
|
||||
return m, revalidateFileViewCmd(target, m.fileView.path, m.fileView.lifetimeToken, m.fileView.loadedSource, m.fileView.liveSeq, m.fileView.loadedSeq)
|
||||
}
|
||||
|
||||
// handleFileViewRevalidated reloads the open view when its source changed after
|
||||
// the displayed snapshot was read. The reload is a refresh, so the shared path
|
||||
// revision advances and the other BTW surface picks up the change as well. If a
|
||||
// newer load replaced the compared snapshot while the read ran, that load may
|
||||
// already show the change, so the view is checked again rather than reloaded.
|
||||
func (m model) handleFileViewRevalidated(msg fileViewRevalidatedMsg) (model, tea.Cmd) {
|
||||
if !msg.changed || !m.fileView.active || m.fileView.mode != fileViewFull ||
|
||||
m.fileView.lifetimeToken != msg.lifetimeToken || m.fileView.path != msg.displayPath {
|
||||
return m, nil
|
||||
}
|
||||
if msg.seq != m.fileView.loadedSeq || !m.fileViewSnapshotCurrent() {
|
||||
return m.revalidateFileView()
|
||||
}
|
||||
return m.startFileViewRefreshCmd(m.chatColumnWidth())
|
||||
}
|
||||
|
||||
// fileViewNavBar renders the single-line header shown in place of the pinned
|
||||
// title bar while the view is active: the path plus the key hints. One line
|
||||
// exactly, so every scrollableTranscriptFrame computed against it agrees with
|
||||
@@ -1327,12 +1448,7 @@ func (m model) renderFileViewFull(width int) string {
|
||||
if !filepath.IsAbs(target) {
|
||||
target = filepath.Join(m.cwd, target)
|
||||
}
|
||||
if m.fileView.snapshotReady &&
|
||||
m.fileView.loadedPath == m.fileView.path &&
|
||||
m.fileView.loadedSeq == m.fileView.desiredSeq &&
|
||||
m.fileView.loadedRev >= m.fileView.requiredSourceRev &&
|
||||
m.fileView.loadedGen == defaultFileViewCache.generation() &&
|
||||
m.fileView.loadedToken == m.fileView.lifetimeToken {
|
||||
if m.fileViewSnapshotCurrent() {
|
||||
if !m.fileView.hasError {
|
||||
defaultFileViewCache.recordHit()
|
||||
}
|
||||
|
||||
@@ -3053,13 +3053,16 @@ func TestFileViewCacheEvictionDropsPathRevision(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// TestFileViewCachePurgeResetsPathRevisions verifies clear() resets the revision
|
||||
// TestFileViewCachePurgeResetsPathRevisions verifies clear() drops the revision
|
||||
// map instead of bumping dead keys forever, so /clear cannot leak one key per
|
||||
// abandoned tool result.
|
||||
// abandoned tool result, while every path's required revision stays monotonic:
|
||||
// a purge never lowers it, and the next invalidation is newer than any
|
||||
// revision issued before the purge.
|
||||
func TestFileViewCachePurgeResetsPathRevisions(t *testing.T) {
|
||||
cache := newFileViewRenderCache(8, defaultFileViewCacheMaxBytes)
|
||||
cache.invalidatePath("one.go")
|
||||
cache.invalidatePath("two.go")
|
||||
lastBefore := cache.invalidatePath("two.go")
|
||||
oneBefore := cache.requiredRevision("one.go")
|
||||
cache.mu.Lock()
|
||||
before := len(cache.pathRevisions)
|
||||
cache.mu.Unlock()
|
||||
@@ -3074,8 +3077,16 @@ func TestFileViewCachePurgeResetsPathRevisions(t *testing.T) {
|
||||
if after != 0 {
|
||||
t.Fatalf("clear() must drop revision keys, got %d", after)
|
||||
}
|
||||
if rev := cache.invalidatePath("one.go"); rev != 1 {
|
||||
t.Fatalf("revision after purge = %d, want 1", rev)
|
||||
for _, p := range []string{"one.go", "two.go", "never_seen.go"} {
|
||||
if got := cache.requiredRevision(p); got < lastBefore {
|
||||
t.Fatalf("required revision for %s fell to %d after purge, below %d", p, got, lastBefore)
|
||||
}
|
||||
}
|
||||
if got := cache.requiredRevision("one.go"); got < oneBefore {
|
||||
t.Fatalf("one.go required revision moved backwards across purge: %d -> %d", oneBefore, got)
|
||||
}
|
||||
if rev := cache.invalidatePath("one.go"); rev <= lastBefore {
|
||||
t.Fatalf("revision after purge = %d, must exceed every pre-purge revision (%d)", rev, lastBefore)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -4223,3 +4234,250 @@ func TestFileViewCrossSurfaceMixedWriteStdinOutsideGit(t *testing.T) {
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
// drainFileViewCommands runs cmd and every command the resulting updates return,
|
||||
// so multi-hop flows (a sweep revalidation that then schedules a reload) settle.
|
||||
func drainFileViewCommands(t *testing.T, m model, cmd tea.Cmd) model {
|
||||
t.Helper()
|
||||
pending := []tea.Cmd{cmd}
|
||||
for steps := 0; len(pending) > 0; steps++ {
|
||||
if steps > 64 {
|
||||
t.Fatal("file-view command chain did not settle")
|
||||
}
|
||||
current := pending[0]
|
||||
pending = pending[1:]
|
||||
if current == nil {
|
||||
continue
|
||||
}
|
||||
msg := current()
|
||||
if msg == nil {
|
||||
continue
|
||||
}
|
||||
if batch, ok := msg.(tea.BatchMsg); ok {
|
||||
pending = append(pending, batch...)
|
||||
continue
|
||||
}
|
||||
updated, next := m.Update(msg)
|
||||
m = updated.(model)
|
||||
pending = append(pending, next)
|
||||
}
|
||||
return m
|
||||
}
|
||||
|
||||
// TestFileViewCrossSurfaceExactEditAfterPurgeReloadsOtherSurface covers a purge
|
||||
// followed by an exact-path edit on the other BTW surface. The purge used to
|
||||
// reset shared path revisions to zero while the side kept its own (higher)
|
||||
// required revision, so the parent's later edit produced a shared revision the
|
||||
// side did not see as newer and the side kept rendering stale content.
|
||||
func TestFileViewCrossSurfaceExactEditAfterPurgeReloadsOtherSurface(t *testing.T) {
|
||||
resetFileViewCacheForTest()
|
||||
dir := t.TempDir()
|
||||
name := "purge_then_edit.go"
|
||||
write := func(content string) {
|
||||
t.Helper()
|
||||
if err := os.WriteFile(filepath.Join(dir, name), []byte(content), 0o644); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
}
|
||||
write("package before\n")
|
||||
|
||||
m := newBTWTestModel(t)
|
||||
m.cwd = dir
|
||||
m.width = 100
|
||||
m.height = 30
|
||||
m.gitFileBaseline = nil
|
||||
m.gitSweepUnavailable = true
|
||||
m.pending = true
|
||||
m.runID = 4
|
||||
m.activeRunID = 4
|
||||
side, _ := m.handleBTWCommand("")
|
||||
if !side.btw.active || side.btw.parent == nil {
|
||||
t.Fatal("fork failed")
|
||||
}
|
||||
side.activeRunID = side.btw.sideRunIDBase
|
||||
side.runID = side.btw.sideRunIDBase
|
||||
side = testOpenFile(side, name)
|
||||
if side.fileView.mode != fileViewFull {
|
||||
t.Fatal("side must open the untouched file in full mode")
|
||||
}
|
||||
|
||||
sideResult := func(tool string, changed []string) {
|
||||
t.Helper()
|
||||
updated, cmd := side.Update(agentRowMsg{
|
||||
runID: side.activeRunID,
|
||||
row: transcriptRow{
|
||||
kind: rowToolResult,
|
||||
tool: tool,
|
||||
status: tools.StatusOK,
|
||||
changedFiles: changed,
|
||||
},
|
||||
})
|
||||
side = drainFileViewCommands(t, updated.(model), cmd)
|
||||
}
|
||||
// An exact side edit gives the path a revision, then an unknown-scope
|
||||
// command purges the shared cache and reloads the side.
|
||||
sideResult("edit_file", []string{name})
|
||||
sideResult("bash", nil)
|
||||
if !strings.Contains(plainRender(t, side.renderFileViewFull(80)), "package before") {
|
||||
t.Fatal("side must settle on the pre-mutation content after the purge reload")
|
||||
}
|
||||
|
||||
write("package after\n")
|
||||
updated, cmd := side.Update(agentRowMsg{
|
||||
runID: 4,
|
||||
row: transcriptRow{
|
||||
kind: rowToolResult,
|
||||
tool: "edit_file",
|
||||
status: tools.StatusOK,
|
||||
changedFiles: []string{name},
|
||||
},
|
||||
})
|
||||
side = drainFileViewCommands(t, updated.(model), cmd)
|
||||
got := plainRender(t, side.renderFileViewFull(80))
|
||||
if !strings.Contains(got, "package after") || strings.Contains(got, "package before") {
|
||||
t.Fatalf("parent edit after a purge left the side stale: %s", got)
|
||||
}
|
||||
}
|
||||
|
||||
// TestFileViewGitSweepRevalidatesWithoutDuplicateReload covers a git sweep that
|
||||
// lands after the open view already reloaded. An unchanged file must keep its
|
||||
// snapshot on screen with no reread-and-highlight pass and no loading
|
||||
// placeholder, while a mutation the sweep cannot attribute to the open path
|
||||
// (the swept list omits it) must still reach the view.
|
||||
func TestFileViewGitSweepRevalidatesWithoutDuplicateReload(t *testing.T) {
|
||||
resetFileViewCacheForTest()
|
||||
dir := t.TempDir()
|
||||
name := "sweep_revalidate.go"
|
||||
filePath := filepath.Join(dir, name)
|
||||
if err := os.WriteFile(filePath, []byte("package v1\n"), 0o644); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
m := filesPanelTestModel()
|
||||
m.cwd = dir
|
||||
m.activeRunID = 1
|
||||
m = testOpenFile(m, name)
|
||||
m = testSetMode(m, fileViewFull)
|
||||
if !strings.Contains(plainRender(t, m.renderFileViewFull(80)), "package v1") {
|
||||
t.Fatal("expected v1")
|
||||
}
|
||||
|
||||
highlights := defaultFileViewCache.stats().HighlightCalls
|
||||
updated, cmd := m.Update(gitSweepMsg{ok: true, files: []gitSweepFile{{path: name, adds: 1}}})
|
||||
m = updated.(model)
|
||||
if got := plainRender(t, m.renderFileViewFull(80)); !strings.Contains(got, "package v1") {
|
||||
t.Fatalf("a sweep must not replace an unchanged snapshot with the loading placeholder: %q", got)
|
||||
}
|
||||
m = drainFileViewCommands(t, m, cmd)
|
||||
if m.fileView.loading {
|
||||
t.Fatal("an unchanged file must not leave the view loading after a sweep")
|
||||
}
|
||||
if after := defaultFileViewCache.stats().HighlightCalls; after != highlights {
|
||||
t.Fatalf("a sweep over an unchanged file re-highlighted it: HighlightCalls %d -> %d", highlights, after)
|
||||
}
|
||||
if got := plainRender(t, m.renderFileViewFull(80)); !strings.Contains(got, "package v1") {
|
||||
t.Fatalf("unchanged snapshot lost after sweep: %q", got)
|
||||
}
|
||||
|
||||
// A later mutation that no tool reported, and that the swept list does not
|
||||
// name, must still be picked up.
|
||||
if err := os.WriteFile(filePath, []byte("package v2\n"), 0o644); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
updated, cmd = m.Update(gitSweepMsg{ok: true})
|
||||
m = drainFileViewCommands(t, updated.(model), cmd)
|
||||
if got := plainRender(t, m.renderFileViewFull(80)); !strings.Contains(got, "package v2") {
|
||||
t.Fatalf("a mutation after the first refresh must reach the view: %q", got)
|
||||
}
|
||||
}
|
||||
|
||||
// TestFileViewGitSweepDuringLoadDefersToThatLoad covers a sweep that lands while
|
||||
// the tool-result refresh is still in flight. The sweep must not cancel and
|
||||
// repeat that load. Once it lands, the view still checks for a mutation made
|
||||
// after the load read the file.
|
||||
func TestFileViewGitSweepDuringLoadDefersToThatLoad(t *testing.T) {
|
||||
resetFileViewCacheForTest()
|
||||
dir := t.TempDir()
|
||||
name := "sweep_inflight.go"
|
||||
filePath := filepath.Join(dir, name)
|
||||
if err := os.WriteFile(filePath, []byte("package v1\n"), 0o644); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
m := filesPanelTestModel()
|
||||
m.cwd = dir
|
||||
m.activeRunID = 1
|
||||
m, loadCmd := m.openFileView(name)
|
||||
if loadCmd == nil || m.fileView.mode != fileViewFull {
|
||||
t.Fatal("expected an in-flight full-file load")
|
||||
}
|
||||
loaded := loadCmd() // reads v1
|
||||
if err := os.WriteFile(filePath, []byte("package v2\n"), 0o644); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
updated, sweepCmd := m.Update(gitSweepMsg{ok: true, files: []gitSweepFile{{path: name}}})
|
||||
m = updated.(model)
|
||||
if sweepCmd != nil {
|
||||
t.Fatal("a sweep during a load must not start a competing load")
|
||||
}
|
||||
updated, next := m.Update(loaded)
|
||||
m = updated.(model)
|
||||
if next == nil {
|
||||
t.Fatal("the landed load must run the deferred sweep revalidation")
|
||||
}
|
||||
m = drainFileViewCommands(t, m, next)
|
||||
if got := plainRender(t, m.renderFileViewFull(80)); !strings.Contains(got, "package v2") {
|
||||
t.Fatalf("a mutation after the in-flight read must reach the view: %q", got)
|
||||
}
|
||||
}
|
||||
|
||||
// TestFileViewStaleRevalidationDoesNotReloadNewerSnapshot covers a revalidation
|
||||
// that saw a change, but landed after a newer load already showed that change.
|
||||
// It must recheck the newer snapshot instead of forcing another reload.
|
||||
func TestFileViewStaleRevalidationDoesNotReloadNewerSnapshot(t *testing.T) {
|
||||
resetFileViewCacheForTest()
|
||||
dir := t.TempDir()
|
||||
name := "stale_revalidation.go"
|
||||
filePath := filepath.Join(dir, name)
|
||||
if err := os.WriteFile(filePath, []byte("package v1\n"), 0o644); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
m := filesPanelTestModel()
|
||||
m.cwd = dir
|
||||
m.activeRunID = 1
|
||||
m = testOpenFile(m, name)
|
||||
if err := os.WriteFile(filePath, []byte("package v2\n"), 0o644); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
updated, sweepCmd := m.Update(gitSweepMsg{ok: true})
|
||||
m = updated.(model)
|
||||
if sweepCmd == nil {
|
||||
t.Fatal("expected a revalidation for the settled snapshot")
|
||||
}
|
||||
stale, ok := sweepCmd().(fileViewRevalidatedMsg)
|
||||
if !ok || !stale.changed {
|
||||
t.Fatalf("revalidation must report the v2 change, got %#v", stale)
|
||||
}
|
||||
|
||||
updated, cmd := m.Update(agentRowMsg{
|
||||
runID: 1,
|
||||
row: transcriptRow{kind: rowToolResult, tool: "edit_file", status: tools.StatusOK, changedFiles: []string{name}},
|
||||
})
|
||||
m = drainFileViewCommands(t, updated.(model), cmd)
|
||||
if got := plainRender(t, m.renderFileViewFull(80)); !strings.Contains(got, "package v2") {
|
||||
t.Fatalf("tool-result refresh must show v2: %q", got)
|
||||
}
|
||||
|
||||
highlights := defaultFileViewCache.stats().HighlightCalls
|
||||
updated, cmd = m.Update(stale)
|
||||
m = updated.(model)
|
||||
if got := plainRender(t, m.renderFileViewFull(80)); !strings.Contains(got, "package v2") {
|
||||
t.Fatalf("a stale revalidation must not replace the current snapshot: %q", got)
|
||||
}
|
||||
m = drainFileViewCommands(t, m, cmd)
|
||||
if m.fileView.loading {
|
||||
t.Fatal("a stale revalidation must not leave the view loading")
|
||||
}
|
||||
if after := defaultFileViewCache.stats().HighlightCalls; after != highlights {
|
||||
t.Fatalf("a stale revalidation re-highlighted an unchanged file: %d -> %d", highlights, after)
|
||||
}
|
||||
}
|
||||
+9
-20
@@ -1456,6 +1456,8 @@ func (m model) updateModel(msg tea.Msg) (tea.Model, tea.Cmd) {
|
||||
switch msg := msg.(type) {
|
||||
case fileViewLoadedMsg:
|
||||
return m.handleFileViewLoaded(msg)
|
||||
case fileViewRevalidatedMsg:
|
||||
return m.handleFileViewRevalidated(msg)
|
||||
case uv.CellSizeEvent:
|
||||
if msg.Width > 0 && msg.Height > 0 {
|
||||
m.petCellPixelWidth = msg.Width
|
||||
@@ -3087,11 +3089,7 @@ func (m model) updateModel(msg tea.Msg) (tea.Model, tea.Cmd) {
|
||||
}
|
||||
rev := defaultFileViewCache.invalidatePath(target)
|
||||
if m.fileView.active && (p == m.fileView.path || target == m.fileView.path) {
|
||||
if rev > m.fileView.requiredSourceRev {
|
||||
m.fileView.requiredSourceRev = rev
|
||||
} else {
|
||||
m.fileView.requiredSourceRev++
|
||||
}
|
||||
m.fileView.requiredSourceRev = rev
|
||||
if m.fileView.mode == fileViewFull {
|
||||
selfNeedsRefresh = true
|
||||
}
|
||||
@@ -3178,21 +3176,12 @@ func (m model) updateModel(msg tea.Msg) (tea.Model, tea.Cmd) {
|
||||
return m, nil
|
||||
case gitSweepMsg:
|
||||
m = m.handleGitSweepMsg(msg)
|
||||
if msg.ok && !msg.baseline {
|
||||
for _, f := range msg.files {
|
||||
p := f.path
|
||||
if !filepath.IsAbs(p) {
|
||||
p = filepath.Join(m.cwd, p)
|
||||
}
|
||||
defaultFileViewCache.invalidatePath(p)
|
||||
}
|
||||
}
|
||||
if m.fileView.active && m.fileView.mode == fileViewFull {
|
||||
var cmd tea.Cmd
|
||||
m, cmd = m.startFileViewRefreshCmd(m.chatColumnWidth())
|
||||
return m, cmd
|
||||
}
|
||||
return m, nil
|
||||
// The swept list is cumulative since the baseline, so it says nothing
|
||||
// about what changed since the open snapshot, and each live sweep starts
|
||||
// beside tool-result, turn-end, or BTW-return handling that already
|
||||
// reloads or rechecks the open view. Revalidate the open view's content
|
||||
// rather than force a second reload.
|
||||
return m.revalidateFileView()
|
||||
case prWatcherStartedMsg:
|
||||
if msg.stop == nil {
|
||||
return m, nil
|
||||
|
||||
@@ -762,10 +762,9 @@ func (m model) handleRewindCommand(args string) (model, string, tea.Cmd) {
|
||||
// Workspace files may be touched or restored starting with ApplyRewind:
|
||||
// Invalidate file view cache immediately so any partial or full disk mutation
|
||||
// cannot serve stale pre-rewind snapshots regardless of subsequent step failures.
|
||||
// The purge advances the cache generation, which retires the open view's
|
||||
// snapshot and any in-flight load without a view-local revision bump.
|
||||
defaultFileViewCache.invalidateUnknownScope()
|
||||
if m.fileView.active {
|
||||
m.fileView.requiredSourceRev++
|
||||
}
|
||||
|
||||
report, err := m.sessionStore.ApplyRewind(m.activeSession.SessionID, m.cwd, target)
|
||||
if err != nil {
|
||||
|
||||
Reference in new issue
Block a user